From b082cc1561a97f2168226b8b9b5714758a07765d Mon Sep 17 00:00:00 2001 From: Widthdom Date: Fri, 5 Jun 2026 13:34:24 +0900 Subject: [PATCH] Fix MCP progress token bounds for #3103 --- changelog.d/unreleased/3103.security.md | 18 +++ src/CodeIndex/Mcp/McpBoundedText.cs | 5 + src/CodeIndex/Mcp/McpServer.cs | 67 ++++++++- src/CodeIndex/Mcp/McpToolDefinitions.cs | 4 +- tests/CodeIndex.Tests/McpServerTests.cs | 172 ++++++++++++++++++++++++ 5 files changed, 261 insertions(+), 5 deletions(-) create mode 100644 changelog.d/unreleased/3103.security.md diff --git a/changelog.d/unreleased/3103.security.md b/changelog.d/unreleased/3103.security.md new file mode 100644 index 0000000000..128c38e61f --- /dev/null +++ b/changelog.d/unreleased/3103.security.md @@ -0,0 +1,18 @@ +--- +category: security +issues: + - 3103 +affected: + - src/CodeIndex/Mcp/McpBoundedText.cs + - src/CodeIndex/Mcp/McpServer.cs + - src/CodeIndex/Mcp/McpToolDefinitions.cs + - tests/CodeIndex.Tests/McpServerTests.cs +--- + +## English + +- **MCP progress tokens are now bounded before echoing (#3103)** — `index` and `backfill_fold` validate `_meta.progressToken` with scalar/object shape, node, depth, string, and serialized-byte budgets before cloning it for progress notifications, ignoring oversized or unsupported tokens instead of amplifying them. + +## 日本語 + +- **MCP progress token を echo 前に bounded にしました (#3103)** — `index` と `backfill_fold` は progress notification 用に clone する前に `_meta.progressToken` を scalar/object 形状、node、depth、string、serialized-byte budget で検証し、上限超過または未対応 token は増幅せず無視します。 diff --git a/src/CodeIndex/Mcp/McpBoundedText.cs b/src/CodeIndex/Mcp/McpBoundedText.cs index d78a292537..baa53e41d3 100644 --- a/src/CodeIndex/Mcp/McpBoundedText.cs +++ b/src/CodeIndex/Mcp/McpBoundedText.cs @@ -26,6 +26,11 @@ internal static class McpBoundedText internal const int MaxPromptNameChars = 128; internal const int MaxPromptArgumentChars = 512; internal const int MaxResourceUriChars = 4096; + internal const int MaxProgressTokenStringChars = 256; + internal const int MaxProgressTokenPropertyNameChars = 64; + internal const int MaxProgressTokenNodeCount = 32; + internal const int MaxProgressTokenDepth = 4; + internal const int MaxProgressTokenJsonBytes = 1024; internal static BoundedMcpText ForDisplay(string value, int maxChars = MaxDiagnosticDisplayChars) { diff --git a/src/CodeIndex/Mcp/McpServer.cs b/src/CodeIndex/Mcp/McpServer.cs index be4114da89..1368c28dc3 100644 --- a/src/CodeIndex/Mcp/McpServer.cs +++ b/src/CodeIndex/Mcp/McpServer.cs @@ -2610,10 +2610,71 @@ private void EmitToolInvocationTelemetry(string toolName, JsonNode? args, JsonNo DeferFrameLog(() => WriteMcpLogLine(evt.ToJsonString(_jsonOptions))); } - private static JsonNode? TryReadProgressToken(JsonNode? callParams) + private JsonNode? TryReadProgressToken(JsonNode? callParams) { var token = callParams?["_meta"]?["progressToken"]; - return token is null ? null : JsonNode.Parse(token.ToJsonString()); + if (token is null) + return null; + + if (!IsSupportedProgressToken(token)) + return null; + + return TryMeasureJsonUtf8BytesWithinLimit(token, _jsonOptions, McpBoundedText.MaxProgressTokenJsonBytes, out _) + ? token.DeepClone() + : null; + } + + private static bool IsSupportedProgressToken(JsonNode token) + { + var nodeCount = 0; + return IsSupportedProgressToken(token, depth: 0, ref nodeCount); + } + + private static bool IsSupportedProgressToken(JsonNode token, int depth, ref int nodeCount) + { + if (depth > McpBoundedText.MaxProgressTokenDepth) + return false; + + nodeCount++; + if (nodeCount > McpBoundedText.MaxProgressTokenNodeCount) + return false; + + return token switch + { + JsonValue value => IsSupportedProgressTokenScalar(value), + JsonObject obj => IsSupportedProgressTokenObject(obj, depth, ref nodeCount), + _ => false, + }; + } + + private static bool IsSupportedProgressTokenScalar(JsonValue value) + => value.GetValueKind() switch + { + JsonValueKind.String => value.TryGetValue(out var text) + && text.Length <= McpBoundedText.MaxProgressTokenStringChars, + JsonValueKind.Number or JsonValueKind.True or JsonValueKind.False => true, + _ => false, + }; + + private static bool IsSupportedProgressTokenObject(JsonObject obj, int depth, ref int nodeCount) + { + foreach (var pair in obj) + { + if (pair.Key.Length > McpBoundedText.MaxProgressTokenPropertyNameChars) + return false; + if (pair.Value is null) + { + nodeCount++; + if (nodeCount > McpBoundedText.MaxProgressTokenNodeCount) + return false; + continue; + } + + if (!IsSupportedProgressToken(pair.Value, depth + 1, ref nodeCount)) + return false; + } + + return true; } private void EmitProgressNotification(JsonNode? progressToken, long progress, long? total, string? message = null) @@ -2623,7 +2684,7 @@ private void EmitProgressNotification(JsonNode? progressToken, long progress, lo var parameters = new JsonObject { - ["progressToken"] = JsonNode.Parse(progressToken.ToJsonString()), + ["progressToken"] = progressToken.DeepClone(), ["progress"] = progress, }; if (total.HasValue) diff --git a/src/CodeIndex/Mcp/McpToolDefinitions.cs b/src/CodeIndex/Mcp/McpToolDefinitions.cs index b903870463..2ee00a335e 100644 --- a/src/CodeIndex/Mcp/McpToolDefinitions.cs +++ b/src/CodeIndex/Mcp/McpToolDefinitions.cs @@ -416,7 +416,7 @@ private JsonNode HandleToolsList(JsonNode? id) ReadOnlyAnnotations()), CreateToolDefinition( "index", - "Index or re-index a project directory. Scans source files, extracts symbols, and builds FTS5 search index. On transports that can carry out-of-band server messages (stdio, and HTTP clients connected to `/events`), when the tools/call request includes `_meta.progressToken`, this tool emits `notifications/progress` with that token while scanning, indexing, and finalizing. / プロジェクトディレクトリをインデックス（再インデックス）。ソースファイルをスキャンし、シンボルを抽出してFTS5検索インデックスを構築。out-of-band のサーバーメッセージを送れる transport（stdio、および `/events` に接続した HTTP クライアント）では、tools/call リクエストに `_meta.progressToken` が含まれる場合、スキャン・インデックス・finalize 中に同じ token の `notifications/progress` を送信する。", + "Index or re-index a project directory. Scans source files, extracts symbols, and builds FTS5 search index. On transports that can carry out-of-band server messages (stdio, and HTTP clients connected to `/events`), when the tools/call request includes a bounded scalar/object `_meta.progressToken`, this tool emits `notifications/progress` with that token while scanning, indexing, and finalizing; oversized or unsupported tokens are ignored instead of echoed. / プロジェクトディレクトリをインデックス（再インデックス）。ソースファイルをスキャンし、シンボルを抽出してFTS5検索インデックスを構築。out-of-band のサーバーメッセージを送れる transport（stdio、および `/events` に接続した HTTP クライアント）では、tools/call リクエストに bounded scalar/object の `_meta.progressToken` が含まれる場合、スキャン・インデックス・finalize 中に同じ token の `notifications/progress` を送信し、上限超過または未対応 token は echo せず無視する。", new JsonObject { ["type"] = "object", @@ -431,7 +431,7 @@ private JsonNode HandleToolsList(JsonNode? id) IndexAnnotations()), CreateToolDefinition( "backfill_fold", - "Upgrade folded-name keys in an existing CodeIndex DB without reparsing source files. Rejects missing or blank targets instead of creating a fresh DB. Fills missing `name_folded` columns (or rewrites all keys after fold metadata drift such as version/fingerprint mismatch) and stamps FoldReady on success. Use `dry_run:true` to preview affected row counts without writing, or `force:true` to rewrite every folded key even when metadata appears current. On transports that can carry out-of-band server messages (stdio, and HTTP clients connected to `/events`), when the tools/call request includes `_meta.progressToken`, this tool emits `notifications/progress` with that token during backfill and verification. / ソース再解析なしで既存の CodeIndex DB の folded-name key を更新する。欠落したDBや空のDBを新規作成せず拒否し、欠損 `name_folded` 列を埋めるか、fold metadata の drift（version / fingerprint 不一致など）時は全 key を再生成し、成功時に FoldReady を stamp する。`dry_run:true` で書き込まず対象行数を確認でき、`force:true` で metadata が current に見える場合でも全 folded key を再生成する。out-of-band のサーバーメッセージを送れる transport（stdio、および `/events` に接続した HTTP クライアント）では、tools/call リクエストに `_meta.progressToken` が含まれる場合、backfill と検証中に同じ token の `notifications/progress` を送信する。", + "Upgrade folded-name keys in an existing CodeIndex DB without reparsing source files. Rejects missing or blank targets instead of creating a fresh DB. Fills missing `name_folded` columns (or rewrites all keys after fold metadata drift such as version/fingerprint mismatch) and stamps FoldReady on success. Use `dry_run:true` to preview affected row counts without writing, or `force:true` to rewrite every folded key even when metadata appears current. On transports that can carry out-of-band server messages (stdio, and HTTP clients connected to `/events`), when the tools/call request includes a bounded scalar/object `_meta.progressToken`, this tool emits `notifications/progress` with that token during backfill and verification; oversized or unsupported tokens are ignored instead of echoed. / ソース再解析なしで既存の CodeIndex DB の folded-name key を更新する。欠落したDBや空のDBを新規作成せず拒否し、欠損 `name_folded` 列を埋めるか、fold metadata の drift（version / fingerprint 不一致など）時は全 key を再生成し、成功時に FoldReady を stamp する。`dry_run:true` で書き込まず対象行数を確認でき、`force:true` で metadata が current に見える場合でも全 folded key を再生成する。out-of-band のサーバーメッセージを送れる transport（stdio、および `/events` に接続した HTTP クライアント）では、tools/call リクエストに bounded scalar/object の `_meta.progressToken` が含まれる場合、backfill と検証中に同じ token の `notifications/progress` を送信し、上限超過または未対応 token は echo せず無視する。", new JsonObject { ["type"] = "object", diff --git a/tests/CodeIndex.Tests/McpServerTests.cs b/tests/CodeIndex.Tests/McpServerTests.cs index 8f64d49d9d..5e38cbdd96 100644 --- a/tests/CodeIndex.Tests/McpServerTests.cs +++ b/tests/CodeIndex.Tests/McpServerTests.cs @@ -2757,6 +2757,178 @@ public async Task RunAsync_IndexWithProgressToken_EmitsProgressNotificationBefor } } + [Fact] + public async Task RunAsync_IndexWithObjectProgressToken_EmitsBoundedClone() + { + var projectRoot = Path.Combine(Directory.GetCurrentDirectory(), $".tmp_mcp_progress_{Guid.NewGuid():N}"); + Directory.CreateDirectory(projectRoot); + try + { + File.WriteAllText(Path.Combine(projectRoot, "one.cs"), "public class One { public void Run() { } }"); + var dbPath = Path.Combine(projectRoot, ".cdidx", "codeindex.db"); + Directory.CreateDirectory(Path.GetDirectoryName(dbPath)!); + using var server = new McpServer(dbPath, "test", dbPathExplicit: true); + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 3103, + ["method"] = "tools/call", + ["params"] = new JsonObject + { + ["name"] = "index", + ["arguments"] = new JsonObject { ["path"] = projectRoot }, + ["_meta"] = new JsonObject + { + ["progressToken"] = new JsonObject + { + ["request"] = "issue-3103", + ["attempt"] = 1, + ["scope"] = new JsonObject { ["tool"] = "index" }, + }, + }, + }, + }; + var transport = new ShutdownProbeTransport("stdio", (Action?)null, request.ToJsonString()); + + await server.RunAsync(transport, CancellationToken.None); + + var progressFrame = transport.WrittenFrames.First(frame => + frame?.Contains("\"method\":\"notifications/progress\"", StringComparison.Ordinal) == true)!; + var progress = JsonNode.Parse(progressFrame)!; + var token = progress["params"]!["progressToken"]!; + Assert.Equal("issue-3103", token["request"]!.GetValue()); + Assert.Equal(1, token["attempt"]!.GetValue()); + Assert.Equal("index", token["scope"]!["tool"]!.GetValue()); + } + finally + { + TestProjectHelper.DeleteDirectory(projectRoot); + } + } + + [Fact] + public async Task RunAsync_IndexWithOversizedProgressToken_ReturnsResultWithoutProgress() + { + var projectRoot = Path.Combine(Directory.GetCurrentDirectory(), $".tmp_mcp_progress_{Guid.NewGuid():N}"); + Directory.CreateDirectory(projectRoot); + try + { + File.WriteAllText(Path.Combine(projectRoot, "one.cs"), "public class One { public void Run() { } }"); + var dbPath = Path.Combine(projectRoot, ".cdidx", "codeindex.db"); + Directory.CreateDirectory(Path.GetDirectoryName(dbPath)!); + using var server = new McpServer(dbPath, "test", dbPathExplicit: true); + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 3104, + ["method"] = "tools/call", + ["params"] = new JsonObject + { + ["name"] = "index", + ["arguments"] = new JsonObject { ["path"] = projectRoot }, + ["_meta"] = new JsonObject + { + ["progressToken"] = new string('x', McpBoundedText.MaxProgressTokenStringChars + 1), + }, + }, + }; + var transport = new ShutdownProbeTransport("stdio", (Action?)null, request.ToJsonString()); + + await server.RunAsync(transport, CancellationToken.None); + + Assert.DoesNotContain(transport.WrittenFrames, frame => + frame?.Contains("\"method\":\"notifications/progress\"", StringComparison.Ordinal) == true); + Assert.Contains(transport.WrittenFrames, frame => + frame?.Contains("\"id\":3104", StringComparison.Ordinal) == true + && frame.Contains("\"structuredContent\"", StringComparison.Ordinal)); + } + finally + { + TestProjectHelper.DeleteDirectory(projectRoot); + } + } + + [Fact] + public async Task RunAsync_IndexWithTooManyProgressTokenNodes_ReturnsResultWithoutProgress() + { + var projectRoot = Path.Combine(Directory.GetCurrentDirectory(), $".tmp_mcp_progress_{Guid.NewGuid():N}"); + Directory.CreateDirectory(projectRoot); + try + { + File.WriteAllText(Path.Combine(projectRoot, "one.cs"), "public class One { public void Run() { } }"); + var dbPath = Path.Combine(projectRoot, ".cdidx", "codeindex.db"); + Directory.CreateDirectory(Path.GetDirectoryName(dbPath)!); + using var server = new McpServer(dbPath, "test", dbPathExplicit: true); + var progressToken = new JsonObject(); + for (var i = 0; i < McpBoundedText.MaxProgressTokenNodeCount; i++) + progressToken[$"k{i}"] = null; + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 3106, + ["method"] = "tools/call", + ["params"] = new JsonObject + { + ["name"] = "index", + ["arguments"] = new JsonObject { ["path"] = projectRoot }, + ["_meta"] = new JsonObject { ["progressToken"] = progressToken }, + }, + }; + var transport = new ShutdownProbeTransport("stdio", (Action?)null, request.ToJsonString()); + + await server.RunAsync(transport, CancellationToken.None); + + Assert.DoesNotContain(transport.WrittenFrames, frame => + frame?.Contains("\"method\":\"notifications/progress\"", StringComparison.Ordinal) == true); + Assert.Contains(transport.WrittenFrames, frame => + frame?.Contains("\"id\":3106", StringComparison.Ordinal) == true + && frame.Contains("\"structuredContent\"", StringComparison.Ordinal)); + } + finally + { + TestProjectHelper.DeleteDirectory(projectRoot); + } + } + + [Fact] + public async Task RunAsync_IndexWithArrayProgressToken_ReturnsResultWithoutProgress() + { + var projectRoot = Path.Combine(Directory.GetCurrentDirectory(), $".tmp_mcp_progress_{Guid.NewGuid():N}"); + Directory.CreateDirectory(projectRoot); + try + { + File.WriteAllText(Path.Combine(projectRoot, "one.cs"), "public class One { public void Run() { } }"); + var dbPath = Path.Combine(projectRoot, ".cdidx", "codeindex.db"); + Directory.CreateDirectory(Path.GetDirectoryName(dbPath)!); + using var server = new McpServer(dbPath, "test", dbPathExplicit: true); + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 3105, + ["method"] = "tools/call", + ["params"] = new JsonObject + { + ["name"] = "index", + ["arguments"] = new JsonObject { ["path"] = projectRoot }, + ["_meta"] = new JsonObject { ["progressToken"] = new JsonArray("unsupported") }, + }, + }; + var transport = new ShutdownProbeTransport("stdio", (Action?)null, request.ToJsonString()); + + await server.RunAsync(transport, CancellationToken.None); + + Assert.DoesNotContain(transport.WrittenFrames, frame => + frame?.Contains("\"method\":\"notifications/progress\"", StringComparison.Ordinal) == true); + Assert.Contains(transport.WrittenFrames, frame => + frame?.Contains("\"id\":3105", StringComparison.Ordinal) == true + && frame.Contains("\"structuredContent\"", StringComparison.Ordinal)); + } + finally + { + TestProjectHelper.DeleteDirectory(projectRoot); + } + } + [Fact] public async Task RunAsync_NonStreamingIndexWithProgressToken_ReturnsFinalResultWithoutProgress() {