From d57595fcc9ca9e7a58255b9253fb800cc3aaa2d9 Mon Sep 17 00:00:00 2001 From: Widthdom Date: Wed, 3 Jun 2026 03:15:39 +0900 Subject: [PATCH 1/3] Bound HTTP MCP request resources (#2815) --- DEVELOPER_GUIDE.md | 16 ++- USER_GUIDE.md | 4 +- changelog.d/unreleased/2815.security.md | 19 ++++ src/CodeIndex/Cli/ProgramRunner.cs | 1 + src/CodeIndex/Mcp/HttpMcpTransport.cs | 107 ++++++++++++++++-- .../CodeIndex.Tests/HttpMcpTransportTests.cs | 80 ++++++++++++- 6 files changed, 210 insertions(+), 17 deletions(-) create mode 100644 changelog.d/unreleased/2815.security.md diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index 9e3c66e05b..c44028203a 100644 --- a/DEVELOPER_GUIDE.md +++ b/DEVELOPER_GUIDE.md @@ -1821,10 +1821,16 @@ return `-32600`. `204 No Content` for notifications. `GET /events` opens an independent `text/event-stream` subscription for future server→client frames; the current server does not yet emit unsolicited frames, but a long-lived - event stream does not block normal POST requests. Non-POST verbs on `/` return - `405 Method Not Allowed`. Empty / whitespace bodies are treated like - a closed stdio line and return `204 No Content` *without* killing the - loop, so a misbehaving client cannot pin the server on a junk frame. + event stream does not block normal POST requests. + Non-POST verbs on `/` return `405 Method Not Allowed`. Empty / whitespace + bodies are treated like a closed stdio line and return `204 No Content` + *without* killing the loop, so a misbehaving client cannot pin the server + on a junk frame. Request bodies are capped by + `CDIDX_MCP_HTTP_MAX_REQUEST_BYTES` (default: 1,000,000 bytes) and oversized + bodies return `413 Payload Too Large` before they are fully buffered. The + pending request queue is bounded by `CDIDX_MCP_HTTP_MAX_QUEUE_DEPTH` + (default: 64); full queues return `429 Too Many Requests` with + `Retry-After: 1` instead of retaining unbounded work. - `ResolveListenSpec("host:port")` resolves the prefix up-front so the CLI can log the bound port to stderr (`Listening on http://...`). Port `0` is resolved by probing a temporary `TcpListener`; the @@ -3130,7 +3136,7 @@ MCP は独立したシリアライズ戦略（オブジェクトを JSON など `HttpMcpTransport`（同じく #1558）は `System.Net.HttpListener` をラップする: -- HTTP POST 1 件 = JSON-RPC フレーム 1 件で、対応する応答は HTTP レスポンスのボディ（`200 OK` / `application/json; charset=utf-8`）に乗る。通知は `204 No Content`。`GET /events` は将来のサーバー→クライアント frame 用に独立した `text/event-stream` subscription を開く。現サーバーは自発的な frame をまだ送信しないが、長寿命の event stream は通常の POST リクエストを塞がない。`/` への POST 以外は `405 Method Not Allowed`。空 / 空白のみのボディは stdio の空行と同じ扱いで `204 No Content` を返し、ループは殺さない — クライアントの誤動作で junk フレームに引っかからないため。 +- HTTP POST 1 件 = JSON-RPC フレーム 1 件で、対応する応答は HTTP レスポンスのボディ（`200 OK` / `application/json; charset=utf-8`）に乗る。通知は `204 No Content`。`GET /events` は将来のサーバー→クライアント frame 用に独立した `text/event-stream` subscription を開く。現サーバーは自発的な frame をまだ送信しないが、長寿命の event stream は通常の POST リクエストを塞がない。`/` への POST 以外は `405 Method Not Allowed`。空 / 空白のみのボディは stdio の空行と同じ扱いで `204 No Content` を返し、ループは殺さない — クライアントの誤動作で junk フレームに引っかからないため。リクエスト本文は `CDIDX_MCP_HTTP_MAX_REQUEST_BYTES`（既定: 1,000,000 bytes）で制限し、超過時は全量を buffer する前に `413 Payload Too Large` を返す。保留中 request queue は `CDIDX_MCP_HTTP_MAX_QUEUE_DEPTH`（既定: 64）で制限し、満杯時は無制限に work を保持せず `Retry-After: 1` 付きの `429 Too Many Requests` を返す。 - `ResolveListenSpec("host:port")` は prefix を事前に解決するため、CLI が stderr に `Listening on http://...` を出せる。ポート `0` は一時 `TcpListener` を probe して空きポートを取得する。probe から `HttpListener.Start()` までの TOCTOU window は、本トランスポートが local-only / single-tenant 想定であるため許容する。ワイルドカードホスト `+` / `*` はパース時点で拒否する。 - 任意の共有秘密による認証: `CDIDX_MCP_HTTP_TOKEN` が設定されていれば、listener はすべてのリクエストに `Authorization: Bearer ` を要求し、定数時間で比較する。トークン未指定で非 loopback ホストへ bind しようとした場合、CLI は MCP カタログを LAN に漏らさないよう既定で拒否する。 - 任意のリクエストループログ: `ProgramRunner` は `HttpMcpTransport` を `GlobalToolLog` に接続するため、lifecycle log が有効な場合は HTTP リクエストごとに `mcp_http_request` 行を 1 件記録する。記録内容は method、path、status、duration、auth outcome、remote peer、correlation id、利用可能な JSON-RPC request id で、リクエスト/レスポンス本文は含めない。 diff --git a/USER_GUIDE.md b/USER_GUIDE.md index 40c8f6323c..cc4f18a1fe 100644 --- a/USER_GUIDE.md +++ b/USER_GUIDE.md @@ -1990,7 +1990,7 @@ CDIDX_MCP_HTTP_TOKEN=s3cret cdidx mcp \ --transport http --http-listen 0.0.0.0:9000 # LAN bind; bearer token is mandatory ``` -Each HTTP `POST /` carries one JSON-RPC frame in the request body, the matching response is returned in the same HTTP body (`200 OK`, `application/json`), and notifications return `204 No Content`. `GET /events` opens a `text/event-stream` channel for server-to-client frames; the current server does not emit unsolicited frames, but the stream is independent and does not block normal POST requests. Non-POST verbs on `/` return `405 Method Not Allowed` with `Allow: POST`. When the persistent lifecycle log is enabled, HTTP mode also writes one `mcp_http_request` record per request with method, path, status, duration, auth outcome, remote peer, correlation id, and JSON-RPC request id when available. Request and response bodies are not logged. +Each HTTP `POST /` carries one JSON-RPC frame in the request body, the matching response is returned in the same HTTP body (`200 OK`, `application/json`), and notifications return `204 No Content`. `GET /events` opens a `text/event-stream` channel for server-to-client frames; the current server does not emit unsolicited frames, but the stream is independent and does not block normal POST requests. Non-POST verbs on `/` return `405 Method Not Allowed` with `Allow: POST`. Request bodies are capped at 1,000,000 bytes by default and oversized requests return `413 Payload Too Large`; the pending POST queue is capped at 64 requests by default and full queues return `429 Too Many Requests` with `Retry-After: 1`. Tune those positive-integer limits with `CDIDX_MCP_HTTP_MAX_REQUEST_BYTES` and `CDIDX_MCP_HTTP_MAX_QUEUE_DEPTH`; invalid values fall back to the defaults. When the persistent lifecycle log is enabled, HTTP mode also writes one `mcp_http_request` record per request with method, path, status, duration, auth outcome, remote peer, correlation id, and JSON-RPC request id when available. Request and response bodies are not logged. Security defaults: @@ -4091,7 +4091,7 @@ CDIDX_MCP_HTTP_TOKEN=s3cret cdidx mcp \ --transport http --http-listen 0.0.0.0:9000 # LAN 公開時は bearer token が必須 ``` -HTTP の `POST /` 1 件が JSON-RPC フレーム 1 件に対応し、応答は同じ HTTP レスポンスのボディに `200 OK` / `application/json` で返ります。通知は `204 No Content` です。`GET /events` はサーバー→クライアントフレーム用の `text/event-stream` channel を開きます。現サーバーは自発的な frame をまだ送信しませんが、この stream は独立しており通常の POST リクエストを塞ぎません。`/` への POST 以外は `405 Method Not Allowed`（`Allow: POST` 付き）です。永続 lifecycle log が有効な場合、HTTP mode はリクエストごとに `mcp_http_request` レコードも出力し、method、path、status、duration、auth outcome、remote peer、correlation id、利用可能な JSON-RPC request id を記録します。リクエスト/レスポンス本文は記録しません。 +HTTP の `POST /` 1 件が JSON-RPC フレーム 1 件に対応し、応答は同じ HTTP レスポンスのボディに `200 OK` / `application/json` で返ります。通知は `204 No Content` です。`GET /events` はサーバー→クライアントフレーム用の `text/event-stream` channel を開きます。現サーバーは自発的な frame をまだ送信しませんが、この stream は独立しており通常の POST リクエストを塞ぎません。`/` への POST 以外は `405 Method Not Allowed`（`Allow: POST` 付き）です。リクエスト本文は既定で 1,000,000 bytes までに制限され、超過時は `413 Payload Too Large` を返します。保留中 POST queue は既定で 64 件までに制限され、満杯時は `Retry-After: 1` 付きの `429 Too Many Requests` を返します。正の整数の `CDIDX_MCP_HTTP_MAX_REQUEST_BYTES` と `CDIDX_MCP_HTTP_MAX_QUEUE_DEPTH` で調整でき、不正値は既定にフォールバックします。永続 lifecycle log が有効な場合、HTTP mode はリクエストごとに `mcp_http_request` レコードも出力し、method、path、status、duration、auth outcome、remote peer、correlation id、利用可能な JSON-RPC request id を記録します。リクエスト/レスポンス本文は記録しません。 セキュリティ既定: diff --git a/changelog.d/unreleased/2815.security.md b/changelog.d/unreleased/2815.security.md new file mode 100644 index 0000000000..792e1b5abf --- /dev/null +++ b/changelog.d/unreleased/2815.security.md @@ -0,0 +1,19 @@ +--- +category: security +issues: + - 2815 +affected: + - src/CodeIndex/Mcp/HttpMcpTransport.cs + - src/CodeIndex/Cli/ProgramRunner.cs + - tests/CodeIndex.Tests/HttpMcpTransportTests.cs + - USER_GUIDE.md + - DEVELOPER_GUIDE.md +--- + +## English + +- **HTTP MCP now bounds request bodies and pending POST queue depth (#2815)** — HTTP requests over the configured body limit now return `413 Payload Too Large`, and a full pending request queue returns `429 Too Many Requests` with `Retry-After: 1` instead of retaining unbounded work. + +## 日本語 + +- **HTTP MCP がリクエスト本文と保留中 POST queue の深さを制限するようになりました (#2815)** — 設定された本文上限を超える HTTP request は `413 Payload Too Large` を返し、保留中 request queue が満杯の場合は work を無制限に保持せず `Retry-After: 1` 付きの `429 Too Many Requests` を返します。 diff --git a/src/CodeIndex/Cli/ProgramRunner.cs b/src/CodeIndex/Cli/ProgramRunner.cs index 4e05025025..91f21c5e30 100644 --- a/src/CodeIndex/Cli/ProgramRunner.cs +++ b/src/CodeIndex/Cli/ProgramRunner.cs @@ -1867,6 +1867,7 @@ private static void PrintMcpUsage() { Console.Error.WriteLine("Usage: cdidx mcp [--db ] [--transport stdio|http] [--http-listen ] [--audit-log ] [--audit-log-include-values] [--audit-log-max-bytes ] [--suggestion-dedup-threshold <0..1>]"); Console.Error.WriteLine("Note: --json is not supported; MCP requests and responses are JSON-RPC over the selected transport."); + Console.Error.WriteLine($"HTTP limits: {HttpMcpTransport.MaxRequestBodyBytesEnvVar}= (default {HttpMcpTransport.DefaultMaxRequestBodyBytes.ToString(CultureInfo.InvariantCulture)}), {HttpMcpTransport.MaxQueueDepthEnvVar}= (default {HttpMcpTransport.DefaultMaxQueuedRequests.ToString(CultureInfo.InvariantCulture)})."); } internal static bool TryConsumeSuggestionDedupThresholdFlag(ref string[] args, out string error) diff --git a/src/CodeIndex/Mcp/HttpMcpTransport.cs b/src/CodeIndex/Mcp/HttpMcpTransport.cs index 9baac60873..b4a324d164 100644 --- a/src/CodeIndex/Mcp/HttpMcpTransport.cs +++ b/src/CodeIndex/Mcp/HttpMcpTransport.cs @@ -26,6 +26,11 @@ namespace CodeIndex.Mcp; /// internal sealed class HttpMcpTransport : IMcpTransport, IOutOfBandMcpTransport { + internal const int DefaultMaxRequestBodyBytes = 1_000_000; + internal const int DefaultMaxQueuedRequests = 64; + internal const string MaxRequestBodyBytesEnvVar = "CDIDX_MCP_HTTP_MAX_REQUEST_BYTES"; + internal const string MaxQueueDepthEnvVar = "CDIDX_MCP_HTTP_MAX_QUEUE_DEPTH"; + private readonly HttpListener _listener; private readonly string _endpoint; private readonly Action? _requestLogger; @@ -33,7 +38,9 @@ internal sealed class HttpMcpTransport : IMcpTransport, IOutOfBandMcpTransport private readonly ConcurrentDictionary _eventStreams = new(); private readonly ConcurrentBag _sseStreams = new(); private readonly CancellationTokenSource _acceptCts = new(); - private readonly Channel _requestQueue = Channel.CreateUnbounded(); + private readonly Channel _requestQueue; + private readonly int _maxRequestBodyBytes; + private readonly int _maxQueuedRequests; private readonly Task _acceptLoop; // The configured bearer token's SHA-256 digest, precomputed once at construction so the // per-request auth path never hashes the secret. Storing the digest (not the token) keeps the @@ -43,6 +50,7 @@ internal sealed class HttpMcpTransport : IMcpTransport, IOutOfBandMcpTransport // 攻撃者入力のみハッシュ計算する。これにより設定トークン長による timing 漏洩を排除する。 private readonly byte[]? _bearerTokenHash; private PendingRequest? _pendingRequest; + private int _queuedRequestCount; private bool _disposed; /// @@ -54,8 +62,24 @@ internal sealed class HttpMcpTransport : IMcpTransport, IOutOfBandMcpTransport /// が空でない場合、すべてのリクエストに `Authorization: Bearer ...` ヘッダーが必要。トークン未指定で /// loopback 以外に bind しようとした場合は明示的に拒否し、秘密情報なしの LAN 露出を防ぐ。 /// - internal HttpMcpTransport(string prefix, string host, int boundPort, string? bearerToken, Action? requestLogger = null) + internal HttpMcpTransport( + string prefix, + string host, + int boundPort, + string? bearerToken, + Action? requestLogger = null, + int? maxRequestBodyBytes = null, + int? maxQueuedRequests = null) { + _maxRequestBodyBytes = ResolvePositiveIntOption(maxRequestBodyBytes, MaxRequestBodyBytesEnvVar, DefaultMaxRequestBodyBytes); + _maxQueuedRequests = ResolvePositiveIntOption(maxQueuedRequests, MaxQueueDepthEnvVar, DefaultMaxQueuedRequests); + _requestQueue = Channel.CreateBounded(new BoundedChannelOptions(_maxQueuedRequests) + { + SingleReader = true, + SingleWriter = false, + FullMode = BoundedChannelFullMode.Wait, + AllowSynchronousContinuations = false, + }); _listener = new HttpListener(); _listener.Prefixes.Add(prefix); _listener.Start(); @@ -83,6 +107,12 @@ internal HttpMcpTransport(string prefix, string host, int boundPort, string? bea internal bool HasEventStreams => !_eventStreams.IsEmpty; + internal int MaxRequestBodyBytes => _maxRequestBodyBytes; + + internal int MaxQueuedRequests => _maxQueuedRequests; + + internal int QueuedRequestCount => Volatile.Read(ref _queuedRequestCount); + /// /// Resolve a `host:port` listen spec into the corresponding HTTP prefix. Ephemeral ports /// (port `0`) are resolved up-front by binding a temporary so the @@ -175,6 +205,22 @@ private static int FindFreePort(IPAddress address) } } + private static int ResolvePositiveIntOption(int? explicitValue, string envVar, int defaultValue) + { + if (explicitValue is { } configured) + { + if (configured <= 0) + throw new ArgumentOutOfRangeException(nameof(explicitValue), configured, "HTTP MCP limits must be positive integers."); + return configured; + } + + var raw = Environment.GetEnvironmentVariable(envVar); + if (int.TryParse(raw, NumberStyles.Integer, CultureInfo.InvariantCulture, out var parsed) && parsed > 0) + return parsed; + + return defaultValue; + } + public async Task ReadFrameAsync(CancellationToken cancellationToken) { ObjectDisposedException.ThrowIf(_disposed, this); @@ -184,6 +230,7 @@ private static int FindFreePort(IPAddress address) try { var request = await _requestQueue.Reader.ReadAsync(cancellationToken).ConfigureAwait(false); + Interlocked.Decrement(ref _queuedRequestCount); _pendingRequest = request; return request.Body; } @@ -267,11 +314,9 @@ private async Task HandleContextAsync(HttpListenerContext context, CancellationT return; } - string body; - using (var reader = new StreamReader(context.Request.InputStream, context.Request.ContentEncoding ?? Encoding.UTF8)) - { - body = await reader.ReadToEndAsync(cancellationToken).ConfigureAwait(false); - } + var body = await TryReadRequestBodyAsync(request, cancellationToken).ConfigureAwait(false); + if (body is null) + return; if (string.IsNullOrWhiteSpace(body)) { @@ -286,7 +331,53 @@ private async Task HandleContextAsync(HttpListenerContext context, CancellationT if (TryHandleOutOfBandFrame(request, body)) return; - await _requestQueue.Writer.WriteAsync(request, cancellationToken).ConfigureAwait(false); + if (!TryQueueRequest(request)) + { + context.Response.AddHeader("Retry-After", "1"); + await RespondAsync(context, (int)HttpStatusCode.TooManyRequests, "MCP HTTP request queue is full.\n").ConfigureAwait(false); + LogRequest(request, (int)HttpStatusCode.TooManyRequests); + } + } + + private async Task TryReadRequestBodyAsync(PendingRequest request, CancellationToken cancellationToken) + { + var context = request.Context; + if (context.Request.ContentLength64 > _maxRequestBodyBytes) + { + await RespondAsync(context, (int)HttpStatusCode.RequestEntityTooLarge, $"MCP HTTP request body exceeds the configured {_maxRequestBodyBytes.ToString(CultureInfo.InvariantCulture)} byte limit.\n").ConfigureAwait(false); + LogRequest(request, (int)HttpStatusCode.RequestEntityTooLarge); + return null; + } + + using var buffer = new MemoryStream(); + var scratch = new byte[Math.Min(8192, _maxRequestBodyBytes)]; + while (true) + { + var read = await context.Request.InputStream.ReadAsync(scratch.AsMemory(), cancellationToken).ConfigureAwait(false); + if (read == 0) + break; + + if (buffer.Length + read > _maxRequestBodyBytes) + { + await RespondAsync(context, (int)HttpStatusCode.RequestEntityTooLarge, $"MCP HTTP request body exceeds the configured {_maxRequestBodyBytes.ToString(CultureInfo.InvariantCulture)} byte limit.\n").ConfigureAwait(false); + LogRequest(request, (int)HttpStatusCode.RequestEntityTooLarge); + return null; + } + + buffer.Write(scratch, 0, read); + } + + return (context.Request.ContentEncoding ?? Encoding.UTF8).GetString(buffer.ToArray()); + } + + private bool TryQueueRequest(PendingRequest request) + { + Interlocked.Increment(ref _queuedRequestCount); + if (_requestQueue.Writer.TryWrite(request)) + return true; + + Interlocked.Decrement(ref _queuedRequestCount); + return false; } private bool TryHandleOutOfBandFrame(PendingRequest request, string body) diff --git a/tests/CodeIndex.Tests/HttpMcpTransportTests.cs b/tests/CodeIndex.Tests/HttpMcpTransportTests.cs index 97a36bd92f..39cf0e48fa 100644 --- a/tests/CodeIndex.Tests/HttpMcpTransportTests.cs +++ b/tests/CodeIndex.Tests/HttpMcpTransportTests.cs @@ -246,6 +246,56 @@ public async Task HttpTransport_EmptyBody_Returns204AndDoesNotKillServer() Assert.Equal(7, doc.RootElement.GetProperty("id").GetInt32()); } + [Fact] + public async Task HttpTransport_RequestBodyOverLimit_Returns413AndDoesNotKillServer() + { + await using var harness = await McpHttpHarness.StartAsync(_dbPath, maxRequestBodyBytes: 64); + + using var oversized = await harness.PostJsonAsync(new string('x', 65)); + + Assert.Equal(HttpStatusCode.RequestEntityTooLarge, oversized.StatusCode); + var rejectedBody = await oversized.Content.ReadAsStringAsync(); + Assert.Contains("64 byte limit", rejectedBody, StringComparison.Ordinal); + + using var follow = await harness.PostJsonAsync("""{"jsonrpc":"2.0","id":7,"method":"ping"}"""); + Assert.Equal(HttpStatusCode.OK, follow.StatusCode); + var body = await follow.Content.ReadAsStringAsync(); + using var doc = JsonDocument.Parse(body); + Assert.Equal(7, doc.RootElement.GetProperty("id").GetInt32()); + } + + [Fact] + public async Task HttpTransport_RequestQueueFull_Returns429() + { + var listen = HttpMcpTransport.ResolveListenSpec("127.0.0.1:0"); + await using var transport = new HttpMcpTransport( + listen.Prefix, + listen.Host, + listen.Port, + bearerToken: null, + maxQueuedRequests: 1); + + using var client = new HttpClient { Timeout = TimeSpan.FromSeconds(5) }; + var first = client.PostAsync( + listen.Prefix, + new StringContent("""{"jsonrpc":"2.0","id":1,"method":"ping"}""", Encoding.UTF8, "application/json")); + await WaitUntilAsync(() => transport.QueuedRequestCount == 1, "the first request to fill the HTTP MCP queue"); + + using var second = await client.PostAsync( + listen.Prefix, + new StringContent("""{"jsonrpc":"2.0","id":2,"method":"ping"}""", Encoding.UTF8, "application/json")); + + Assert.Equal(HttpStatusCode.TooManyRequests, second.StatusCode); + Assert.Contains(second.Headers, header => header.Key == "Retry-After"); + + var frame = await transport.ReadFrameAsync(CancellationToken.None).WaitAsync(TimeSpan.FromSeconds(5)); + Assert.NotNull(frame); + Assert.Contains("\"id\":1", frame, StringComparison.Ordinal); + await transport.WriteFrameAsync("""{"jsonrpc":"2.0","id":1,"result":{}}""", CancellationToken.None); + using var firstResponse = await first.WaitAsync(TimeSpan.FromSeconds(5)); + Assert.Equal(HttpStatusCode.OK, firstResponse.StatusCode); + } + [Fact] public async Task HttpTransport_EventsStream_DoesNotBlockPostRequests() { @@ -514,6 +564,20 @@ private static async Task ReadUntilAsync(StreamReader reader, string exp return builder.ToString(); } + private static async Task WaitUntilAsync(Func condition, string description) + { + var deadline = DateTimeOffset.UtcNow.AddSeconds(5); + while (DateTimeOffset.UtcNow < deadline) + { + if (condition()) + return; + + await Task.Delay(10); + } + + Assert.Fail($"Timed out waiting for {description}."); + } + private sealed class McpHttpHarness : IAsyncDisposable { private static readonly TimeSpan RequestTimeout = TimeSpan.FromSeconds(30); @@ -533,10 +597,22 @@ private McpHttpHarness(McpServer server, HttpMcpTransport transport, Cancellatio public string Endpoint { get; } - public static async Task StartAsync(string dbPath, string? bearerToken = null, Action? requestLogger = null) + public static async Task StartAsync( + string dbPath, + string? bearerToken = null, + Action? requestLogger = null, + int? maxRequestBodyBytes = null, + int? maxQueuedRequests = null) { var listen = HttpMcpTransport.ResolveListenSpec("127.0.0.1:0"); - var transport = new HttpMcpTransport(listen.Prefix, listen.Host, listen.Port, bearerToken, requestLogger); + var transport = new HttpMcpTransport( + listen.Prefix, + listen.Host, + listen.Port, + bearerToken, + requestLogger, + maxRequestBodyBytes, + maxQueuedRequests); var server = new McpServer(dbPath, ConsoleUi.LoadVersion()); var cts = new CancellationTokenSource(); var loopTask = Task.Run(() => server.RunAsync(transport, cts.Token)); From 7bb4bc8270946a693589be94602de750ef6a69bd Mon Sep 17 00:00:00 2001 From: Widthdom Date: Wed, 3 Jun 2026 03:17:24 +0900 Subject: [PATCH 2/3] Stop retaining HTTP MCP SSE stream tasks (#2823) --- DEVELOPER_GUIDE.md | 3 +++ changelog.d/unreleased/2823.security.md | 17 +++++++++++++++++ src/CodeIndex/Mcp/HttpMcpTransport.cs | 3 +-- .../CodeIndex.Tests/HttpMcpTransportTests.cs | 19 +++++++++++++++++++ 4 files changed, 40 insertions(+), 2 deletions(-) create mode 100644 changelog.d/unreleased/2823.security.md diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index c44028203a..f310f89335 100644 --- a/DEVELOPER_GUIDE.md +++ b/DEVELOPER_GUIDE.md @@ -1831,6 +1831,8 @@ return `-32600`. pending request queue is bounded by `CDIDX_MCP_HTTP_MAX_QUEUE_DEPTH` (default: 64); full queues return `429 Too Many Requests` with `Retry-After: 1` instead of retaining unbounded work. +- SSE stream lifetime is represented by the active stream registry only; + completed stream tasks are not retained after that registry entry is removed. - `ResolveListenSpec("host:port")` resolves the prefix up-front so the CLI can log the bound port to stderr (`Listening on http://...`). Port `0` is resolved by probing a temporary `TcpListener`; the @@ -3137,6 +3139,7 @@ MCP は独立したシリアライズ戦略（オブジェクトを JSON など `HttpMcpTransport`（同じく #1558）は `System.Net.HttpListener` をラップする: - HTTP POST 1 件 = JSON-RPC フレーム 1 件で、対応する応答は HTTP レスポンスのボディ（`200 OK` / `application/json; charset=utf-8`）に乗る。通知は `204 No Content`。`GET /events` は将来のサーバー→クライアント frame 用に独立した `text/event-stream` subscription を開く。現サーバーは自発的な frame をまだ送信しないが、長寿命の event stream は通常の POST リクエストを塞がない。`/` への POST 以外は `405 Method Not Allowed`。空 / 空白のみのボディは stdio の空行と同じ扱いで `204 No Content` を返し、ループは殺さない — クライアントの誤動作で junk フレームに引っかからないため。リクエスト本文は `CDIDX_MCP_HTTP_MAX_REQUEST_BYTES`（既定: 1,000,000 bytes）で制限し、超過時は全量を buffer する前に `413 Payload Too Large` を返す。保留中 request queue は `CDIDX_MCP_HTTP_MAX_QUEUE_DEPTH`（既定: 64）で制限し、満杯時は無制限に work を保持せず `Retry-After: 1` 付きの `429 Too Many Requests` を返す。 +- SSE stream lifetime は active stream registry だけで表現し、その registry entry が削除された後に完了済み stream task を保持しない。 - `ResolveListenSpec("host:port")` は prefix を事前に解決するため、CLI が stderr に `Listening on http://...` を出せる。ポート `0` は一時 `TcpListener` を probe して空きポートを取得する。probe から `HttpListener.Start()` までの TOCTOU window は、本トランスポートが local-only / single-tenant 想定であるため許容する。ワイルドカードホスト `+` / `*` はパース時点で拒否する。 - 任意の共有秘密による認証: `CDIDX_MCP_HTTP_TOKEN` が設定されていれば、listener はすべてのリクエストに `Authorization: Bearer ` を要求し、定数時間で比較する。トークン未指定で非 loopback ホストへ bind しようとした場合、CLI は MCP カタログを LAN に漏らさないよう既定で拒否する。 - 任意のリクエストループログ: `ProgramRunner` は `HttpMcpTransport` を `GlobalToolLog` に接続するため、lifecycle log が有効な場合は HTTP リクエストごとに `mcp_http_request` 行を 1 件記録する。記録内容は method、path、status、duration、auth outcome、remote peer、correlation id、利用可能な JSON-RPC request id で、リクエスト/レスポンス本文は含めない。 diff --git a/changelog.d/unreleased/2823.security.md b/changelog.d/unreleased/2823.security.md new file mode 100644 index 0000000000..bc5af8cda0 --- /dev/null +++ b/changelog.d/unreleased/2823.security.md @@ -0,0 +1,17 @@ +--- +category: security +issues: + - 2823 +affected: + - src/CodeIndex/Mcp/HttpMcpTransport.cs + - tests/CodeIndex.Tests/HttpMcpTransportTests.cs + - DEVELOPER_GUIDE.md +--- + +## English + +- **HTTP MCP SSE streams no longer retain completed stream tasks (#2823)** — event streams are tracked only through the active stream registry, and disconnected streams are removed without keeping completed `Task` references for the process lifetime. + +## 日本語 + +- **HTTP MCP SSE stream が完了済み stream task を保持しなくなりました (#2823)** — event stream は active stream registry だけで追跡され、切断済み stream はプロセス寿命いっぱい完了済み `Task` 参照を保持せずに削除されます。 diff --git a/src/CodeIndex/Mcp/HttpMcpTransport.cs b/src/CodeIndex/Mcp/HttpMcpTransport.cs index b4a324d164..b10977ccd8 100644 --- a/src/CodeIndex/Mcp/HttpMcpTransport.cs +++ b/src/CodeIndex/Mcp/HttpMcpTransport.cs @@ -36,7 +36,6 @@ internal sealed class HttpMcpTransport : IMcpTransport, IOutOfBandMcpTransport private readonly Action? _requestLogger; private readonly object _requestLoggerGate = new(); private readonly ConcurrentDictionary _eventStreams = new(); - private readonly ConcurrentBag _sseStreams = new(); private readonly CancellationTokenSource _acceptCts = new(); private readonly Channel _requestQueue; private readonly int _maxRequestBodyBytes; @@ -302,7 +301,7 @@ private async Task HandleContextAsync(HttpListenerContext context, CancellationT return; } - _sseStreams.Add(Task.Run(() => RunEventStreamAsync(request, cancellationToken), CancellationToken.None)); + _ = Task.Run(() => RunEventStreamAsync(request, cancellationToken), CancellationToken.None); return; } diff --git a/tests/CodeIndex.Tests/HttpMcpTransportTests.cs b/tests/CodeIndex.Tests/HttpMcpTransportTests.cs index 39cf0e48fa..3f5ee11348 100644 --- a/tests/CodeIndex.Tests/HttpMcpTransportTests.cs +++ b/tests/CodeIndex.Tests/HttpMcpTransportTests.cs @@ -335,6 +335,23 @@ public async Task HttpTransport_EventsStream_EmitsOptInKeepAliveNotifications() Assert.Contains("\"uptime_s\":", frame, StringComparison.Ordinal); } + [Fact] + public async Task HttpTransport_EventsStream_RemovesDisconnectedStreams() + { + using var env = EnvironmentVariableScope.Capture("CDIDX_MCP_KEEP_ALIVE_INTERVAL_S"); + env.Set("CDIDX_MCP_KEEP_ALIVE_INTERVAL_S", "0.02"); + await using var harness = await McpHttpHarness.StartAsync(_dbPath); + + using var client = new HttpClient(); + using var events = await client.GetAsync(new Uri(new Uri(harness.Endpoint), "events"), HttpCompletionOption.ResponseHeadersRead); + Assert.Equal(HttpStatusCode.OK, events.StatusCode); + await WaitUntilAsync(() => harness.HasEventStreams, "the event stream to be registered"); + + events.Dispose(); + + await WaitUntilAsync(() => !harness.HasEventStreams, "the disconnected event stream to be removed"); + } + [Fact] public async Task HttpTransport_IndexWithProgressToken_EmitsProgressOnEventsStreamAndReturnsResult() { @@ -597,6 +614,8 @@ private McpHttpHarness(McpServer server, HttpMcpTransport transport, Cancellatio public string Endpoint { get; } + public bool HasEventStreams => _transport.HasEventStreams; + public static async Task StartAsync( string dbPath, string? bearerToken = null, From 3ac275380a4d1e6538ed463d24c63d426f487a8f Mon Sep 17 00:00:00 2001 From: Widthdom Date: Wed, 3 Jun 2026 03:20:45 +0900 Subject: [PATCH 3/3] Expire idle MCP rate limiter buckets (#2824) --- DEVELOPER_GUIDE.md | 2 + USER_GUIDE.md | 2 + changelog.d/unreleased/2824.security.md | 18 +++++ src/CodeIndex/Mcp/RateLimiter.cs | 96 ++++++++++++++++++++++- tests/CodeIndex.Tests/RateLimiterTests.cs | 94 ++++++++++++++++++++++ 5 files changed, 210 insertions(+), 2 deletions(-) create mode 100644 changelog.d/unreleased/2824.security.md diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index f310f89335..9dbe649cad 100644 --- a/DEVELOPER_GUIDE.md +++ b/DEVELOPER_GUIDE.md @@ -1181,6 +1181,7 @@ Process exit codes are coarse (`0` success including valid zero-row queries, `1` - **Authoritative C# metadata-target trust** — `deps` / `impact` metadata-attribute edges (linking `[Foo]` usage to the defining `FooAttribute` class) are promoted from a signature-shape heuristic to an authoritative resolver whenever `is_metadata_target` is persisted under the current `metadata_target_version_csharp` contract. The resolver walks C# class base lists with fixed-point transitive resolution through same-DB class rows and falls back to the BCL `Attribute` suffix convention only for unresolved external bases. Readiness lives in `codeindex_meta`, and the reader uses a three-way branch: (1) ready → `is_metadata_target = 1`; (2) column present but not stamped (legacy row) → `signature LIKE '%: %'`; (3) column missing → naming-only fallback. This fixes non-attribute impostors (`class FooAttribute : BaseService`) silently dropping edges when they shared names with real `FooAttribute : Attribute` classes (#435). - **Human-readable default** — All commands default to human-readable output. `--json` for AI/machine consumption. - **Structured MCP responses** — MCP tool calls return typed JSON in `structuredContent` and keep `content` concise for compatibility. +- **MCP rate limiter bucket eviction** — `RateLimiter` keeps `(tool, caller)` token buckets only while they are active. `CDIDX_MCP_RATE_LIMIT_BUCKET_IDLE_SECONDS` defaults to 900 seconds; stale buckets are pruned on later acquisitions so shared or HTTP MCP deployments do not retain historical caller identities for the process lifetime (#2824). - **MCP `batch_query` response cap** — `batch_query` estimates the UTF-8 JSON size of aggregate slot results and stops appending once the response would exceed `CDIDX_MCP_BATCH_RESPONSE_MAX_BYTES` (default: 1,000,000 bytes, aligned with the JSON-RPC line cap). Truncated responses include `truncated: true`, `truncated_queries`, and byte-limit metadata so clients can split the batch or lower per-slot limits without parsing prose (#1416). - **MCP array argument bounds** — MCP string-array filters such as `path`, `project`, `excludePaths`, and mixed `names` arrays reject invalid entries instead of silently dropping them. Arrays are capped at 100 entries and each entry is capped at 4096 characters; `batch_query` reports these validation failures per slot with `request_index` and `ok: false`. - **MCP schema lock-down** — Every tool `inputSchema` includes `additionalProperties: false`, and `tools/call` mirrors that contract by rejecting unknown argument names with `-32602` / `invalid_argument` instead of silently defaulting misspelled fields. @@ -2794,6 +2795,7 @@ USER_GUIDEの[終了コード](USER_GUIDE.md#終了コード)セクションを exact な SQL の graph/dependency reader は解決済み segment 数も保持するため、`"sales.fn_Target"` のようなドット入り quoted single identifier が、本物の qualified name `sales.fn_Target` と exact `references` / `callers` / `impact` や集計系の `deps` / `unused` / `hotspots` で衝突しない。 - **言語考慮の参照抽出** — `references`、`callers`、`callees` は、正規表現ベースの call/reference 抽出が意味を持つ言語だけに対してインデックス化された参照テーブルで支える。未対応言語では、低信頼な疑似グラフ結果を返す代わりにテキスト検索へ戻る前提で設計する。**nested generic 呼び出し**: `new Dictionary>()` のような C#/Java のコンストラクタ呼び出しと、`Helper.DoWork>()` のような C# generic method call は、平坦な regex fast-path で `>>` を釣り合わせられなくても depth-aware fallback scanner で拾い直し、外側 target を参照テーブルへ残す。**JS/TS の no-paren constructor**: JavaScript / TypeScript の zero-argument constructor call で `()` を合法的に省略できる `new Foo;`、`new Date;`、`new Demo.Provider;`、`new Box;` も、専用の言語別経路で `instantiate` edge として出す。行末 `new Foo` に対する次行 `.bar()` / `[0]` continuation は suppress し、phantom な単独 instantiation にしない。**コンストラクタ連鎖呼び出し**: C# の `: this(...)` / `: base(...)` イニシャライザと、Java のコンストラクタ本体冒頭文 `this(...)` / `super(...)` は、汎用 call regex とは別に検出し、呼び先が実際のコンストラクタとなるように書き換える（`this` は外側の class/record、`base` / `super` は外側クラスのシグネチャから解析した基底型）。C# のクロス行イニシャライザは外側クラスではなく、そのコンストラクタに紐付ける。基底型の解析は generic 引数、record のプライマリコンストラクタ引数、`where` 制約、`global::` やドット付きの namespace 修飾を剥がす。Java の `super.method()` は通常のメソッド呼び出しのまま扱う。**型位置の依存エッジ**: C#/Java の継承リスト、宣言型、generic 制約、`throws`、`is` / `as` / `instanceof`、および C# XML doc の `cref` は `type_reference` 行として索引し、既定の `callers` / `callees` が見せる動的 call graph を汚さずに、`references` / `impact` から compile-time rename 依存を辿れるようにする。C# XML doc の `cref` 抽出は、実際に後続宣言へ結び付く XML-doc comment である `///` 行と delimited `/** ... */` block の両方を対象にしつつ、通常の `//` / `////` コメントや通常の block comment は phantom 依存として扱わない。また、同じ物理行でも closing `*/` より後ろに続く code / string の内容、doc comment と後続宣言の間へ割り込むトップレベル実行文、brace-free field/property initializer continuation、brace-free expression lambda、nested executable continuation、複数行 raw/verbatim string のうち行頭がたまたま `/**` で始まる内容は doc-comment slice の外として扱う。regex 自体は narrowed した doc-comment slice に対して走らせるが、`symbol_references.column` は元の物理ソース行位置に固定したまま保持する。C# の read path では、`using static` による constant-pattern suppress が `is` / `case` の前後の trivia を考慮してトークン単位で判定され、anchor が前行にある場合は anchor-aware な複数行コンテキストをインデックス済み行から再構成するため、`value is/*comment*/Red`、`value is\n Red or Blue`、`value is\n // comment\n Red`、`case\n // comment\n Point:`、長い `case` / `or` 連鎖、`case\tRed:` のような形でも phantom `type_reference` を漏らさない。qualified constant/member pattern は exact-name read path でも qualifier 起点で suppress するため、`case Color.Red or Color.Blue:` に対して無関係な `class Red {}` が suppress を打ち消さない。extractor 側の pending type-pattern carry も trivia-only 区切り行、standalone な continuation-line `not`、複数行 `case` head / logical continuation をまたいで維持されるため、comment-only 行や `not` だけの継続行で後続の本物の type head を落とさない。`case > 0:` や `case not > 0:` のような非型 `case` ラベルではその pending carry を armed にしないため、次行の call/identifier token が `type_reference` に混入しない。同名型の rescue も `file` 可視性を尊重し、file-local な型は同じ物理ファイル内の参照だけを救済する。基底クラスから見える protected/public/internal nested type は、基底型参照を active な型 alias / namespace alias 経由まで正規化し、さらに alias 展開後に constructed generic な基底型を再 canonicalize したうえで derived class の pattern head を救済する一方、implemented interface は inherited nested-type rescue に参加しない。さらに same-file `using Namespace;`、project-wide `global using Namespace;`、型 alias も同じ rescue 集合に入る。一方で extractor は file-local な情報だけでは同一 namespace の別ファイルにある実型を判定できないため、`value is Red` のような曖昧な unqualified `using static` head は DB に残し、pure constant-only case の抑止は workspace-aware な read path 側で行う。**SQL qualified-name alignment**: SQL の graph/dependency reader は、各 reference 行の source-line context、記録済み call 列位置、enclosing container から SQL 参照名を復元して定義と照合するため、qualified な `references` / `callers` / `impact` query は exact / non-exact を問わず sibling schema へ widen しない。source 側が genuinely unqualified な場合にだけ bare leaf fallback を許可するので、qualified call を含む `deps` / `unused` / `hotspots` も schema 単位で整合し、`EXEC dbo.fn_Target; EXEC sales.fn_Target;` のような同一行 multi-call も二重計上しない。列位置が記録されている row は、その列に qualified token が見つからなければ whole-line の別 qualified token へ昇格させないため、行末コメント・文字列リテラル・後続の別 call が先頭の unqualified edge を横取りすることもない。qualified な `callees` query でも caller query 自体が unqualified なとき以外は leaf fallback を無効化したため、`callees sales.Caller` が `dbo.Caller` へ広がらない。SQL extractor は qualified-name の `.` 前後空白も許容し、definition 系 reader は quoted qualified SQL name (`[dbo].[fn_X]` → `dbo.fn_X`) を正規化してから照合する。さらに exact SQL 定義照合は segment 数を保持し、SQL の exact graph leaf fallback は Unicode folded exact path を維持する。SQL CTE 本体内の source 行は raw `cte_body_reference` kind を使うため、`references --kind cte_body_reference` で anchor/recursive member 内部を outer query の table reference と区別できる。そのため、quoted single identifier の衝突や Unicode exact lookup の ASCII-only `NOCASE` 退行も防ぐ。exact な SQL の graph/dependency reader は解決済み segment 数も保持するため、`"sales.fn_Target"` のようなドット入り quoted single identifier が、本物の qualified name `sales.fn_Target` と exact `references` / `callers` / `impact` や集計系の `deps` / `unused` / `hotspots` で衝突しない。 - **構造化MCPレスポンス** — MCPツール呼び出しは `structuredContent` に型付きJSONを返し、`content` は互換性のため簡潔に保つ。 +- **MCP rate limiter bucket eviction** — `RateLimiter` は active な `(tool, caller)` token bucket だけを保持する。`CDIDX_MCP_RATE_LIMIT_BUCKET_IDLE_SECONDS` は既定 900 秒で、古い bucket は後続 acquisition 時に pruning されるため、共有または HTTP MCP デプロイが過去の caller ID をプロセス寿命いっぱい保持しない (#2824)。 - **MCP `batch_query` レスポンス上限** — `batch_query` は集約した slot 結果の UTF-8 JSON サイズを見積もり、`CDIDX_MCP_BATCH_RESPONSE_MAX_BYTES`（既定: JSON-RPC 行上限と揃えた 1,000,000 bytes）を超える場合は追加を止める。切り詰めたレスポンスには `truncated: true`、`truncated_queries`、byte limit メタデータを含めるため、クライアントは prose を parsing せず batch 分割や slot limit 縮小を判断できる (#1416)。 - **MCP 配列引数の上限** — `path` / `project` / `excludePaths` / mixed `names` などの string-array filter は、不正要素を暗黙に落とさず拒否する。配列は 100 件、各要素は 4096 文字を上限とし、`batch_query` では `request_index` と `ok: false` 付きの slot 失敗として報告する。 - **MCP schema のロックダウン** — すべての tool `inputSchema` は `additionalProperties: false` を含み、`tools/call` も同じ契約として未知の引数名を黙って既定値にせず `-32602` / `invalid_argument` で拒否する。 diff --git a/USER_GUIDE.md b/USER_GUIDE.md index cc4f18a1fe..71b6dfabe5 100644 --- a/USER_GUIDE.md +++ b/USER_GUIDE.md @@ -1364,6 +1364,7 @@ Example output: |---|---| | `CDIDX_MCP_RATE_LIMIT_RPS` | Refill rate in tokens per second. Required to enable rate limiting; values that are missing, non-numeric, zero, negative, or non-finite (`Infinity`, `NaN`) leave the limiter disabled and emit a one-line warning on `stderr`. | | `CDIDX_MCP_RATE_LIMIT_BURST` | Bucket capacity (maximum burst). Optional. Defaults to `max(rps, 1)`. Invalid or non-finite values fall back to the default and emit a warning while leaving `rps` honored. | +| `CDIDX_MCP_RATE_LIMIT_BUCKET_IDLE_SECONDS` | Idle bucket TTL. Optional. Defaults to 900 seconds. Stale `(tool, caller)` buckets are pruned on later calls so long-lived servers do not retain historical caller identities forever. Invalid or non-finite values fall back to the default and emit a warning. | Caller identity is captured from the `clientInfo.name` (and `version` when present) of the MCP `initialize` request. Tool calls received before `initialize` are billed against an anonymous `"unknown"` bucket so an unidentified client cannot bypass the limiter. The captured caller is sticky for the lifetime of the session — once a named identity has been recorded, subsequent `initialize` calls under a different name are ignored (with a one-line `stderr` warning) so a long-lived stdio or networked session cannot reset its bucket mid-flight by re-identifying. @@ -3484,6 +3485,7 @@ MCP ツールで catch-all まで突き抜けた例外（想定外の SQLite 例 |---|---| | `CDIDX_MCP_RATE_LIMIT_RPS` | 1 秒あたりのトークン補充レート。レート制限を有効化するために必須。未設定・非数値・0 以下・非有限値（`Infinity`/`NaN`）の場合は無効のまま、1 行の警告を `stderr` に出力します。 | | `CDIDX_MCP_RATE_LIMIT_BURST` | バケット容量（最大バースト）。任意。既定は `max(rps, 1)`。不正値・非有限値は既定にフォールバックし警告を出力。`rps` はそのまま尊重されます。 | +| `CDIDX_MCP_RATE_LIMIT_BUCKET_IDLE_SECONDS` | 未使用バケットの TTL。任意。既定は 900 秒です。古い `(tool, caller)` バケットは後続呼び出し時に pruning され、長時間稼働するサーバーが過去の caller ID を永続保持しません。不正値・非有限値は既定にフォールバックし警告を出力します。 | 呼び出し元 ID は MCP `initialize` リクエストの `clientInfo.name`（および `version` があれば併記）から取得します。`initialize` 前に届いたツール呼び出しは匿名 `"unknown"` バケットで計量され、未識別クライアントによる制限回避を防ぎます。取得済みの caller はセッション中 sticky で、名前付き ID が一度記録されると以降の別名 `initialize` は無視され（`stderr` に 1 行警告）、長期 stdio / 通信セッションが途中で再 initialize してバケットをリセットする経路を塞ぎます。 diff --git a/changelog.d/unreleased/2824.security.md b/changelog.d/unreleased/2824.security.md new file mode 100644 index 0000000000..68d141e53c --- /dev/null +++ b/changelog.d/unreleased/2824.security.md @@ -0,0 +1,18 @@ +--- +category: security +issues: + - 2824 +affected: + - src/CodeIndex/Mcp/RateLimiter.cs + - tests/CodeIndex.Tests/RateLimiterTests.cs + - USER_GUIDE.md + - DEVELOPER_GUIDE.md +--- + +## English + +- **MCP rate limiter buckets now expire after an idle TTL (#2824)** — stale `(tool, caller)` token buckets are pruned on later acquisitions, with `CDIDX_MCP_RATE_LIMIT_BUCKET_IDLE_SECONDS` defaulting to 900 seconds so long-running servers do not retain historical caller identities forever. + +## 日本語 + +- **MCP rate limiter bucket が idle TTL 後に期限切れになるようになりました (#2824)** — 古い `(tool, caller)` token bucket は後続 acquisition 時に pruning され、`CDIDX_MCP_RATE_LIMIT_BUCKET_IDLE_SECONDS` の既定値 900 秒により長時間稼働するサーバーが過去の caller ID を永続保持しません。 diff --git a/src/CodeIndex/Mcp/RateLimiter.cs b/src/CodeIndex/Mcp/RateLimiter.cs index b1c9879a32..6e4001ffa7 100644 --- a/src/CodeIndex/Mcp/RateLimiter.cs +++ b/src/CodeIndex/Mcp/RateLimiter.cs @@ -15,6 +15,7 @@ internal sealed class RateLimiter private readonly Dictionary _buckets = new(StringComparer.Ordinal); private readonly RateLimiterOptions _options; private readonly Func _clock; + private DateTimeOffset _nextPruneAt = DateTimeOffset.MinValue; public RateLimiter(RateLimiterOptions options, Func? clock = null) { @@ -24,6 +25,17 @@ public RateLimiter(RateLimiterOptions options, Func? clock = nul public RateLimiterOptions Options => _options; + internal int BucketCount + { + get + { + lock (_gate) + { + return _buckets.Count; + } + } + } + /// /// Try to take one token from the (tool, caller) bucket. When rate limiting is disabled /// (the env-driven default), the call is always allowed and the limiter performs no @@ -43,6 +55,7 @@ public RateLimiterDecision TryAcquire(string tool, string caller) var now = _clock(); lock (_gate) { + PruneIdleBuckets(now); if (!_buckets.TryGetValue(key, out var bucket)) { bucket = new TokenBucket(_options.BurstCapacity, now); @@ -54,17 +67,74 @@ public RateLimiterDecision TryAcquire(string tool, string caller) internal static string BuildKey(string tool, string caller) => $"{tool}|{caller}"; + private void PruneIdleBuckets(DateTimeOffset now) + { + var idleTtl = _options.BucketIdleTtl; + if (idleTtl <= TimeSpan.Zero || now < _nextPruneAt) + return; + + var cutoff = ComputeIdleCutoff(now, idleTtl); + List? expiredKeys = null; + foreach (var (key, bucket) in _buckets) + { + if (bucket.LastTouched <= cutoff) + (expiredKeys ??= new List()).Add(key); + } + + if (expiredKeys is not null) + { + foreach (var key in expiredKeys) + _buckets.Remove(key); + } + + _nextPruneAt = ComputeNextPruneAt(now, idleTtl); + } + + private static TimeSpan ComputePruneInterval(TimeSpan idleTtl) + { + var interval = TimeSpan.FromTicks(Math.Max(TimeSpan.TicksPerMillisecond, idleTtl.Ticks / 4)); + return interval <= TimeSpan.FromMinutes(1) ? interval : TimeSpan.FromMinutes(1); + } + + private static DateTimeOffset ComputeIdleCutoff(DateTimeOffset now, TimeSpan idleTtl) + { + try + { + return now - idleTtl; + } + catch (ArgumentOutOfRangeException) + { + return DateTimeOffset.MinValue; + } + } + + private static DateTimeOffset ComputeNextPruneAt(DateTimeOffset now, TimeSpan idleTtl) + { + try + { + return now + ComputePruneInterval(idleTtl); + } + catch (ArgumentOutOfRangeException) + { + return DateTimeOffset.MaxValue; + } + } + private sealed class TokenBucket { private double _tokens; private DateTimeOffset _lastUpdate; + private DateTimeOffset _lastTouched; public TokenBucket(double initialTokens, DateTimeOffset createdAt) { _tokens = initialTokens; _lastUpdate = createdAt; + _lastTouched = createdAt; } + public DateTimeOffset LastTouched => _lastTouched; + public RateLimiterDecision TryAcquire(DateTimeOffset now, double refillRate, double capacity) { // Defense in depth: the public surface gates on RateLimiterOptions.IsEnabled @@ -83,6 +153,8 @@ public RateLimiterDecision TryAcquire(DateTimeOffset now, double refillRate, dou _tokens = Math.Min(capacity, _tokens + elapsedSeconds * refillRate); _lastUpdate = now; } + if (now > _lastTouched) + _lastTouched = now; // When elapsedSeconds <= 0 (clock drift / repeated tick / backwards step) we // intentionally do NOT touch _lastUpdate. The bucket stays anchored to its // previous base, so the next forward tick computes elapsed against the older @@ -125,14 +197,17 @@ internal readonly record struct RateLimiterDecision(bool Allowed, long RetryAfte /// internal sealed class RateLimiterOptions { + internal static readonly TimeSpan DefaultBucketIdleTtl = TimeSpan.FromMinutes(15); internal const string RpsEnvVar = "CDIDX_MCP_RATE_LIMIT_RPS"; internal const string BurstEnvVar = "CDIDX_MCP_RATE_LIMIT_BURST"; + internal const string BucketIdleSecondsEnvVar = "CDIDX_MCP_RATE_LIMIT_BUCKET_IDLE_SECONDS"; public double RefillTokensPerSecond { get; init; } public double BurstCapacity { get; init; } + public TimeSpan BucketIdleTtl { get; init; } = DefaultBucketIdleTtl; public bool IsEnabled => RefillTokensPerSecond > 0 && BurstCapacity > 0; - public static RateLimiterOptions Disabled { get; } = new() { RefillTokensPerSecond = 0, BurstCapacity = 0 }; + public static RateLimiterOptions Disabled { get; } = new() { RefillTokensPerSecond = 0, BurstCapacity = 0, BucketIdleTtl = DefaultBucketIdleTtl }; public static RateLimiterOptions FromEnvironment(Func? envReader = null, Action? warningSink = null) { @@ -165,7 +240,12 @@ public static RateLimiterOptions FromEnvironment(Func? envReade burst = Math.Max(rps, 1.0); } - return new RateLimiterOptions { RefillTokensPerSecond = rps, BurstCapacity = burst }; + var bucketIdleTtl = DefaultBucketIdleTtl; + var bucketIdleRaw = envReader(BucketIdleSecondsEnvVar); + if (!string.IsNullOrWhiteSpace(bucketIdleRaw) && !TryParsePositiveTimeSpanSeconds(bucketIdleRaw, out bucketIdleTtl)) + warningSink($"[cdidx-mcp] Ignoring invalid {BucketIdleSecondsEnvVar}='{bucketIdleRaw}'. Expected a positive finite number of seconds. Falling back to the default bucket idle TTL."); + + return new RateLimiterOptions { RefillTokensPerSecond = rps, BurstCapacity = burst, BucketIdleTtl = bucketIdleTtl }; } private static bool TryParsePositiveDouble(string raw, out double value) @@ -178,4 +258,16 @@ private static bool TryParsePositiveDouble(string raw, out double value) value = 0; return false; } + + private static bool TryParsePositiveTimeSpanSeconds(string raw, out TimeSpan value) + { + if (TryParsePositiveDouble(raw, out var seconds) && seconds <= TimeSpan.MaxValue.TotalSeconds) + { + value = TimeSpan.FromSeconds(seconds); + return true; + } + + value = DefaultBucketIdleTtl; + return false; + } } diff --git a/tests/CodeIndex.Tests/RateLimiterTests.cs b/tests/CodeIndex.Tests/RateLimiterTests.cs index d63428af6f..d652c393f0 100644 --- a/tests/CodeIndex.Tests/RateLimiterTests.cs +++ b/tests/CodeIndex.Tests/RateLimiterTests.cs @@ -156,6 +156,66 @@ public void Clock_GoingBackwards_DoesNotOverRefill() Assert.True(limiter.TryAcquire("search", "client-a").Allowed); } + [Fact] + public void IdleBuckets_ArePrunedAfterConfiguredTtl() + { + var clock = new TestClock(); + var options = new RateLimiterOptions + { + RefillTokensPerSecond = 1.0, + BurstCapacity = 1.0, + BucketIdleTtl = TimeSpan.FromSeconds(1), + }; + var limiter = new RateLimiter(options, clock.Read); + + Assert.True(limiter.TryAcquire("search", "client-a").Allowed); + Assert.Equal(1, limiter.BucketCount); + + clock.Now = clock.Now.AddMilliseconds(500); + Assert.True(limiter.TryAcquire("search", "client-b").Allowed); + Assert.Equal(2, limiter.BucketCount); + + clock.Now = clock.Now.AddSeconds(2); + Assert.True(limiter.TryAcquire("search", "client-c").Allowed); + Assert.Equal(1, limiter.BucketCount); + } + + [Fact] + public void IdleBuckets_LargeTtl_DoesNotUnderflowCutoff() + { + var clock = new TestClock(); + var options = new RateLimiterOptions + { + RefillTokensPerSecond = 1.0, + BurstCapacity = 10.0, + BucketIdleTtl = TimeSpan.MaxValue, + }; + var limiter = new RateLimiter(options, clock.Read); + + Assert.True(limiter.TryAcquire("search", "client-a").Allowed); + Assert.Equal(1, limiter.BucketCount); + + clock.Now = clock.Now.AddDays(1); + Assert.True(limiter.TryAcquire("search", "client-b").Allowed); + Assert.Equal(2, limiter.BucketCount); + } + + [Fact] + public void IdleBuckets_NearMaxClock_DoesNotOverflowNextPrune() + { + var clock = new TestClock { Now = DateTimeOffset.MaxValue.AddMilliseconds(-10) }; + var options = new RateLimiterOptions + { + RefillTokensPerSecond = 1.0, + BurstCapacity = 10.0, + BucketIdleTtl = TimeSpan.FromSeconds(1), + }; + var limiter = new RateLimiter(options, clock.Read); + + Assert.True(limiter.TryAcquire("search", "client-a").Allowed); + Assert.Equal(1, limiter.BucketCount); + } + [Fact] public void FromEnvironment_NoVars_ReturnsDisabled() { @@ -201,6 +261,40 @@ public void FromEnvironment_ExplicitBurst_IsHonored() Assert.Equal(20.0, opts.BurstCapacity); } + [Fact] + public void FromEnvironment_BucketIdleSeconds_IsHonored() + { + var opts = RateLimiterOptions.FromEnvironment( + key => key switch + { + RateLimiterOptions.RpsEnvVar => "2", + RateLimiterOptions.BurstEnvVar => "4", + RateLimiterOptions.BucketIdleSecondsEnvVar => "30", + _ => null, + }, + _ => { }); + Assert.True(opts.IsEnabled); + Assert.Equal(TimeSpan.FromSeconds(30), opts.BucketIdleTtl); + } + + [Fact] + public void FromEnvironment_InvalidBucketIdleSeconds_WarnsAndFallsBack() + { + var warnings = new List(); + var opts = RateLimiterOptions.FromEnvironment( + key => key switch + { + RateLimiterOptions.RpsEnvVar => "2", + RateLimiterOptions.BucketIdleSecondsEnvVar => "NaN", + _ => null, + }, + warnings.Add); + Assert.True(opts.IsEnabled); + Assert.Equal(RateLimiterOptions.DefaultBucketIdleTtl, opts.BucketIdleTtl); + Assert.Single(warnings); + Assert.Contains("CDIDX_MCP_RATE_LIMIT_BUCKET_IDLE_SECONDS", warnings[0]); + } + [Fact] public void FromEnvironment_InvalidRps_WarnsAndDisables() {