From 5ddab0dac57c4768067afde98779d8ee6040b693 Mon Sep 17 00:00:00 2001 From: Widthdom Date: Thu, 4 Jun 2026 22:03:50 +0900 Subject: [PATCH 01/11] Bound MCP enum scalar diagnostics (#3116) --- changelog.d/unreleased/3116.security.md | 17 +++++++ src/CodeIndex/Mcp/McpBoundedText.cs | 41 +++++++++++++++++ src/CodeIndex/Mcp/McpToolHandlers.cs | 59 ++++++++++++++++++++++++- tests/CodeIndex.Tests/McpServerTests.cs | 41 +++++++++++++++++ 4 files changed, 157 insertions(+), 1 deletion(-) create mode 100644 changelog.d/unreleased/3116.security.md create mode 100644 src/CodeIndex/Mcp/McpBoundedText.cs diff --git a/changelog.d/unreleased/3116.security.md b/changelog.d/unreleased/3116.security.md new file mode 100644 index 0000000000..bc28dcf276 --- /dev/null +++ b/changelog.d/unreleased/3116.security.md @@ -0,0 +1,17 @@ +--- +category: security +issues: + - 3116 +affected: + - src/CodeIndex/Mcp/McpBoundedText.cs + - src/CodeIndex/Mcp/McpToolHandlers.cs + - tests/CodeIndex.Tests/McpServerTests.cs +--- + +## English + +- **MCP enum-like scalar arguments are bounded before normalization (#3116)** — oversized `format`, `groupBy`, and related scalar option values are rejected before trimming or lowercasing, and diagnostics echo only a bounded display value with truncation metadata. + +## 日本語 + +- **MCP の enum 風 scalar 引数を正規化前に制限するようになりました (#3116)** — `format`、`groupBy` などの scalar option 値が大きすぎる場合は trim / lowercase の前に拒否し、診断には切り詰めた表示値と truncation metadata だけを返します。 diff --git a/src/CodeIndex/Mcp/McpBoundedText.cs b/src/CodeIndex/Mcp/McpBoundedText.cs new file mode 100644 index 0000000000..34a2a71355 --- /dev/null +++ b/src/CodeIndex/Mcp/McpBoundedText.cs @@ -0,0 +1,41 @@ +using System.Text; +using System.Text.Json.Nodes; + +namespace CodeIndex.Mcp; + +internal readonly record struct BoundedMcpText(string Text, int OriginalLength, bool Truncated) +{ + internal void AddMetadata(JsonObject target, string prefix) + { + if (!Truncated) + return; + + target[$"{prefix}_length"] = OriginalLength; + target[$"{prefix}_truncated"] = true; + } +} + +internal static class McpBoundedText +{ + internal const int MaxScalarArgumentChars = 512; + internal const int MaxDiagnosticDisplayChars = 128; + + internal static BoundedMcpText ForDisplay(string value, int maxChars = MaxDiagnosticDisplayChars) + { + ArgumentNullException.ThrowIfNull(value); + ArgumentOutOfRangeException.ThrowIfNegative(maxChars); + + var truncated = value.Length > maxChars; + var displayLength = Math.Min(value.Length, maxChars); + var sb = new StringBuilder(displayLength + (truncated ? 3 : 0)); + for (var i = 0; i < displayLength; i++) + { + var ch = value[i]; + sb.Append(ch < 0x20 || ch == 0x7F ? '?' : ch); + } + if (truncated) + sb.Append("..."); + + return new BoundedMcpText(sb.ToString(), value.Length, truncated); + } +} diff --git a/src/CodeIndex/Mcp/McpToolHandlers.cs b/src/CodeIndex/Mcp/McpToolHandlers.cs index d18e58845f..fe149c50fd 100644 --- a/src/CodeIndex/Mcp/McpToolHandlers.cs +++ b/src/CodeIndex/Mcp/McpToolHandlers.cs @@ -25,6 +25,17 @@ public partial class McpServer private const string BatchQueryResponseByteLimitEnvVar = "CDIDX_MCP_BATCH_RESPONSE_MAX_BYTES"; internal const int MaxMcpArrayFilterCount = 100; internal const int MaxMcpArrayFilterStringLength = 4096; + private static readonly HashSet BoundedEnumLikeScalarArguments = new(StringComparer.Ordinal) + { + "category", + "direction", + "format", + "groupBy", + "kind", + "lang", + "language", + "rankBy", + }; // --- Tool implementations / ツール実装 --- @@ -523,6 +534,37 @@ private static List ReadStringList(JsonNode? args, string propertyName) if (ValidateToolArgumentTypes(toolName, obj) is JsonObject typeError) return typeError; + if (ValidateBoundedEnumLikeScalarArguments(toolName, obj) is JsonObject scalarError) + return scalarError; + + return null; + } + + private static JsonObject? ValidateBoundedEnumLikeScalarArguments(string toolName, JsonObject args) + { + foreach (var property in args) + { + if (!BoundedEnumLikeScalarArguments.Contains(property.Key)) + continue; + if (property.Value is not JsonValue value || !value.TryGetValue(out var scalar)) + continue; + if (scalar.Length <= McpBoundedText.MaxScalarArgumentChars) + continue; + + var display = McpBoundedText.ForDisplay(scalar); + var error = new JsonObject + { + ["message"] = $"Argument '{property.Key}' on tool '{toolName}' is too long (max {McpBoundedText.MaxScalarArgumentChars} characters): '{display.Text}'.", + ["tool"] = toolName, + ["parameter"] = property.Key, + ["value"] = display.Text, + ["max_length"] = McpBoundedText.MaxScalarArgumentChars, + ["actual_length"] = display.OriginalLength, + }; + display.AddMetadata(error, "value"); + return error; + } + return null; } @@ -3193,7 +3235,22 @@ private JsonNode ExecuteSymbolHotspots(JsonNode? id, JsonNode? args) var groupBy = args?["groupBy"]?.GetValue()?.ToLowerInvariant() ?? (string.Equals(lang, "sql", StringComparison.Ordinal) ? "statement" : "symbol"); if (groupBy is not ("symbol" or "file" or "statement")) - return CreateToolErrorResponse(id, $"Unsupported symbol_hotspots groupBy '{groupBy}'. Use symbol, file, or statement."); + { + var groupByDisplay = McpBoundedText.ForDisplay(groupBy); + var extra = new JsonObject + { + ["parameter"] = "groupBy", + ["value"] = groupByDisplay.Text, + }; + groupByDisplay.AddMetadata(extra, "value"); + return CreateToolErrorResponse( + id, + $"Unsupported symbol_hotspots groupBy '{groupByDisplay.Text}'. Use symbol, file, or statement.", + category: McpErrorEnvelope.CategoryInvalidArgument, + suggestion: "Use symbol, file, or statement for symbol_hotspots groupBy.", + retrySafe: false, + extraData: extra); + } var pathPatterns = ReadScopedPathList(args); var excludePaths = ReadStringList(args, "excludePaths"); var excludeTests = args?["excludeTests"]?.GetValue() ?? false; diff --git a/tests/CodeIndex.Tests/McpServerTests.cs b/tests/CodeIndex.Tests/McpServerTests.cs index b633515cff..94d63d0df3 100644 --- a/tests/CodeIndex.Tests/McpServerTests.cs +++ b/tests/CodeIndex.Tests/McpServerTests.cs @@ -213,6 +213,47 @@ public void ToolsCall_SearchFormatCountAliasesCountOnly_Issue1642() Assert.Empty(structured["results"]!.AsArray()); } + [Theory] + [InlineData("search", "format")] + [InlineData("symbol_hotspots", "groupBy")] + public void ToolsCall_EnumLikeScalarTooLong_RejectsBeforeNormalization_Issue3116(string toolName, string argumentName) + { + var oversized = new string('A', McpBoundedText.MaxScalarArgumentChars + 1); + var args = new JsonObject + { + [argumentName] = oversized, + }; + if (toolName == "search") + args["query"] = "Run"; + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 1, + ["method"] = "tools/call", + ["params"] = new JsonObject + { + ["name"] = toolName, + ["arguments"] = args, + }, + }; + + var response = _server.HandleMessage(request)!; + + var result = response["result"]!; + Assert.True(result["isError"]!.GetValue()); + var text = result["content"]![0]!["text"]!.GetValue(); + var structured = result["structuredContent"]!; + Assert.Contains($"Argument '{argumentName}' on tool '{toolName}' is too long", text); + Assert.DoesNotContain(oversized, response.ToJsonString(), StringComparison.Ordinal); + Assert.Equal(toolName, structured["tool"]!.GetValue()); + Assert.Equal(argumentName, structured["parameter"]!.GetValue()); + Assert.Equal(McpBoundedText.MaxScalarArgumentChars, structured["max_length"]!.GetValue()); + Assert.Equal(oversized.Length, structured["actual_length"]!.GetValue()); + Assert.True(structured["value_truncated"]!.GetValue()); + Assert.Equal(oversized.Length, structured["value_length"]!.GetValue()); + Assert.Equal(McpBoundedText.ForDisplay(oversized).Text, structured["value"]!.GetValue()); + } + [Fact] public void ToolsCall_SearchReturnsStableAtAndCursorContinuesAfterAnchor_Issue1462() { From c2cae6ea913ba75267e0ad9588f044cf6ee80949 Mon Sep 17 00:00:00 2001 From: Widthdom Date: Thu, 4 Jun 2026 22:08:57 +0900 Subject: [PATCH 02/11] Truncate MCP unknown argument names (#3117) --- changelog.d/unreleased/3117.security.md | 17 ++++++++ src/CodeIndex/Mcp/McpServer.cs | 5 ++- src/CodeIndex/Mcp/McpToolHandlers.cs | 34 ++++++++++------ tests/CodeIndex.Tests/McpServerTests.cs | 52 +++++++++++++++++++++++++ 4 files changed, 94 insertions(+), 14 deletions(-) create mode 100644 changelog.d/unreleased/3117.security.md diff --git a/changelog.d/unreleased/3117.security.md b/changelog.d/unreleased/3117.security.md new file mode 100644 index 0000000000..a2c504a8e8 --- /dev/null +++ b/changelog.d/unreleased/3117.security.md @@ -0,0 +1,17 @@ +--- +category: security +issues: + - 3117 +affected: + - src/CodeIndex/Mcp/McpServer.cs + - src/CodeIndex/Mcp/McpToolHandlers.cs + - tests/CodeIndex.Tests/McpServerTests.cs +--- + +## English + +- **MCP unknown argument diagnostics truncate property names (#3117)** — unknown argument names now use bounded display values in errors and audit/telemetry key sketches, with original length metadata when truncation occurs. + +## 日本語 + +- **MCP の未知argument診断でproperty名を切り詰めるようになりました (#3117)** — 未知argument名はエラーおよび audit / telemetry のkey概要でbounded display値を使い、切り詰め時は元の長さmetadataも返します。 diff --git a/src/CodeIndex/Mcp/McpServer.cs b/src/CodeIndex/Mcp/McpServer.cs index 53efee1e2c..50de0a25dc 100644 --- a/src/CodeIndex/Mcp/McpServer.cs +++ b/src/CodeIndex/Mcp/McpServer.cs @@ -2632,8 +2632,9 @@ internal static (IReadOnlyList Keys, IReadOnlyList>(argsObj.Count); foreach (var (key, value) in argsObj) { - keys.Add(key); - lengths.Add(new KeyValuePair(key, AuditLogSink.MeasureArgLength(value))); + var keyDisplay = McpBoundedText.ForDisplay(key); + keys.Add(keyDisplay.Text); + lengths.Add(new KeyValuePair(keyDisplay.Text, AuditLogSink.MeasureArgLength(value))); } JsonNode? echo = null; diff --git a/src/CodeIndex/Mcp/McpToolHandlers.cs b/src/CodeIndex/Mcp/McpToolHandlers.cs index fe149c50fd..ae7b1e7ec5 100644 --- a/src/CodeIndex/Mcp/McpToolHandlers.cs +++ b/src/CodeIndex/Mcp/McpToolHandlers.cs @@ -510,23 +510,25 @@ private static List ReadStringList(JsonNode? args, string propertyName) var allowed = GetAllowedToolArguments(toolName); if (allowed.Count == 0) - return obj.Count == 0 ? null : new JsonObject - { - ["message"] = $"Tool '{toolName}' does not accept arguments.", - ["tool"] = toolName, - ["unknown_argument"] = obj.First().Key, - }; + return obj.Count == 0 ? null : AddUnknownArgumentData( + new JsonObject + { + ["message"] = $"Tool '{toolName}' does not accept arguments.", + ["tool"] = toolName, + }, + obj.First().Key); foreach (var property in obj) { if (!allowed.Contains(property.Key)) { - return new JsonObject - { - ["message"] = $"Unknown argument '{property.Key}' for tool '{toolName}'.", - ["tool"] = toolName, - ["unknown_argument"] = property.Key, - }; + return AddUnknownArgumentData( + new JsonObject + { + ["message"] = $"Unknown argument '{McpBoundedText.ForDisplay(property.Key).Text}' for tool '{toolName}'.", + ["tool"] = toolName, + }, + property.Key); } } @@ -568,6 +570,14 @@ private static List ReadStringList(JsonNode? args, string propertyName) return null; } + private static JsonObject AddUnknownArgumentData(JsonObject error, string argumentName) + { + var display = McpBoundedText.ForDisplay(argumentName); + error["unknown_argument"] = display.Text; + display.AddMetadata(error, "unknown_argument"); + return error; + } + private static JsonObject? ValidateToolArgumentTypes(string toolName, JsonObject args) { foreach (var property in args) diff --git a/tests/CodeIndex.Tests/McpServerTests.cs b/tests/CodeIndex.Tests/McpServerTests.cs index 94d63d0df3..4ff8d3a9a6 100644 --- a/tests/CodeIndex.Tests/McpServerTests.cs +++ b/tests/CodeIndex.Tests/McpServerTests.cs @@ -7302,6 +7302,58 @@ public void ToolsCall_Index_RejectsUnsupportedArguments_Issue2848(string argumen Assert.Equal(argumentName, structured["unknown_argument"]!.GetValue()); } + [Fact] + public void ToolsCall_UnknownArgumentName_TruncatesDisplay_Issue3117() + { + var argumentName = new string('x', McpBoundedText.MaxDiagnosticDisplayChars + 25); + var display = McpBoundedText.ForDisplay(argumentName); + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 1, + ["method"] = "tools/call", + ["params"] = new JsonObject + { + ["name"] = "index", + ["arguments"] = new JsonObject + { + ["path"] = ".", + [argumentName] = true, + }, + }, + }; + + var response = _server.HandleMessage(request)!; + + Assert.True(response["result"]!["isError"]!.GetValue()); + Assert.DoesNotContain(argumentName, response.ToJsonString(), StringComparison.Ordinal); + var text = response["result"]!["content"]![0]!["text"]!.GetValue(); + Assert.Contains($"Unknown argument '{display.Text}' for tool 'index'.", text); + var structured = response["result"]!["structuredContent"]!; + Assert.Equal(display.Text, structured["unknown_argument"]!.GetValue()); + Assert.Equal(argumentName.Length, structured["unknown_argument_length"]!.GetValue()); + Assert.True(structured["unknown_argument_truncated"]!.GetValue()); + } + + [Fact] + public void SanitizeArgs_TruncatesArgumentKeysForAuditAndTelemetry_Issue3117() + { + var argumentName = new string('k', McpBoundedText.MaxDiagnosticDisplayChars + 1); + var display = McpBoundedText.ForDisplay(argumentName); + var args = new JsonObject + { + [argumentName] = "value", + }; + + var (keys, lengths, valuesEcho) = McpServer.SanitizeArgs(args, includeValues: false); + + Assert.Equal([display.Text], keys); + var length = Assert.Single(lengths); + Assert.Equal(display.Text, length.Key); + Assert.Equal("value".Length, length.Value); + Assert.Null(valuesEcho); + } + [Fact] public void ToolsCall_Index_WhenDbLockHeld_ReturnsBusyError() { From b483b4813183c3595b830ec9272aa36e75569b4e Mon Sep 17 00:00:00 2001 From: Widthdom Date: Thu, 4 Jun 2026 22:38:07 +0900 Subject: [PATCH 03/11] Bound MCP unknown tool diagnostics (#3118) --- changelog.d/unreleased/3118.security.md | 19 ++++ src/CodeIndex/Mcp/McpBoundedText.cs | 1 + src/CodeIndex/Mcp/McpServer.cs | 80 ++++++++++--- src/CodeIndex/Mcp/McpToolHandlers.cs | 21 +++- tests/CodeIndex.Tests/McpAuditLogTests.cs | 32 ++++++ tests/CodeIndex.Tests/McpServerTests.cs | 132 ++++++++++++++++++++++ 6 files changed, 261 insertions(+), 24 deletions(-) create mode 100644 changelog.d/unreleased/3118.security.md diff --git a/changelog.d/unreleased/3118.security.md b/changelog.d/unreleased/3118.security.md new file mode 100644 index 0000000000..fd70f5eb6a --- /dev/null +++ b/changelog.d/unreleased/3118.security.md @@ -0,0 +1,19 @@ +--- +category: security +issues: + - 3118 +affected: + - src/CodeIndex/Mcp/McpBoundedText.cs + - src/CodeIndex/Mcp/McpServer.cs + - src/CodeIndex/Mcp/McpToolHandlers.cs + - tests/CodeIndex.Tests/McpServerTests.cs + - tests/CodeIndex.Tests/McpAuditLogTests.cs +--- + +## English + +- **MCP unknown tool diagnostics truncate tool names (#3118)** — unknown tool names now use bounded display values in JSON-RPC errors, batch slot results, telemetry, and audit logs, with original length metadata when truncation occurs. + +## 日本語 + +- **MCP の未知tool診断でtool名を切り詰めるようになりました (#3118)** — 未知tool名は JSON-RPC error、batch slot結果、telemetry、audit log で bounded display 値を使い、切り詰め時は元の長さmetadataも返します。 diff --git a/src/CodeIndex/Mcp/McpBoundedText.cs b/src/CodeIndex/Mcp/McpBoundedText.cs index 34a2a71355..e54bb42a65 100644 --- a/src/CodeIndex/Mcp/McpBoundedText.cs +++ b/src/CodeIndex/Mcp/McpBoundedText.cs @@ -19,6 +19,7 @@ internal static class McpBoundedText { internal const int MaxScalarArgumentChars = 512; internal const int MaxDiagnosticDisplayChars = 128; + internal const int MaxToolNameChars = 128; internal static BoundedMcpText ForDisplay(string value, int maxChars = MaxDiagnosticDisplayChars) { diff --git a/src/CodeIndex/Mcp/McpServer.cs b/src/CodeIndex/Mcp/McpServer.cs index 50de0a25dc..f5a2f3a96e 100644 --- a/src/CodeIndex/Mcp/McpServer.cs +++ b/src/CodeIndex/Mcp/McpServer.cs @@ -2253,6 +2253,7 @@ private async Task HandleToolsCallAsync(JsonNode? id, JsonNode? callPa TryEmitAudit("(missing)", id, args, missingNameResponse, DateTimeOffset.UtcNow, 0.0, errorType: "missing_tool_name"); return missingNameResponse; } + var toolNameTooLong = toolName.Length > McpBoundedText.MaxToolNameChars; // Per-deployment enablement gate (#1561). Disabled known tools return `-32601 method // not found` so clients can branch on a structured JSON-RPC code; truly unknown names @@ -2291,7 +2292,11 @@ private async Task HandleToolsCallAsync(JsonNode? id, JsonNode? callPa JsonNode response; try { - if (ValidateToolArguments(toolName, args) is JsonObject argumentError) + if (toolNameTooLong) + { + response = CreateUnknownToolErrorResponse(hasId: true, id: id, toolName); + } + else if (ValidateToolArguments(toolName, args) is JsonObject argumentError) { metricsError = "invalid_argument"; if (argumentError["jsonrpc_invalid_params"] is JsonValue invalidParamsMarker @@ -2366,11 +2371,7 @@ private async Task HandleToolsCallAsync(JsonNode? id, JsonNode? callPa "index" => await ExecuteIndexAsync(id, args, progressToken).ConfigureAwait(false), "backfill_fold" => ExecuteBackfillFold(id, args, progressToken), "suggest_improvement" => await ExecuteSuggestImprovementAsync(id, args).ConfigureAwait(false), - _ => CreateErrorResponse(hasId: true, id: id, code: -32602, message: $"Unknown tool: {toolName}", - category: McpErrorEnvelope.CategoryToolUnknown, - suggestion: "Call tools/list to enumerate the available tool names for this server. Tool name match is case-sensitive.", - retrySafe: false, - extraData: new JsonObject { ["tool"] = toolName }), + _ => CreateUnknownToolErrorResponse(hasId: true, id: id, toolName), }; } } @@ -2403,11 +2404,7 @@ private async Task HandleToolsCallAsync(JsonNode? id, JsonNode? callPa category: classification.Category, suggestion: classification.Suggestion, retrySafe: classification.RetrySafe, - extraData: new JsonObject - { - ["tool"] = toolName, - ["exception_type"] = ex.GetType().Name, - }); + extraData: BuildToolExceptionData(toolName, ex.GetType().Name)); } finally { @@ -2444,6 +2441,7 @@ private void EmitToolInvocationTelemetry(string toolName, JsonNode? args, JsonNo var (errorCode, observedErrorType) = ExtractErrorCode(response); var resultCount = ExtractResultCount(response); var (argKeys, argLengths, _) = SanitizeArgs(args, includeValues: false); + var toolDisplay = BoundToolNameForDisplay(toolName); var argsObject = new JsonObject(); foreach (var pair in argLengths) argsObject[pair.Key] = pair.Value; @@ -2452,7 +2450,7 @@ private void EmitToolInvocationTelemetry(string toolName, JsonNode? args, JsonNo { ["event"] = "mcp.tool.invocation", ["timestamp"] = startedAt.ToString("O", System.Globalization.CultureInfo.InvariantCulture), - ["tool"] = toolName, + ["tool"] = toolDisplay.Text, ["request_id"] = context?.RequestId, ["correlation_id"] = context?.CorrelationId, ["elapsed_ms"] = Math.Round(elapsedMs, 3), @@ -2463,6 +2461,7 @@ private void EmitToolInvocationTelemetry(string toolName, JsonNode? args, JsonNo ["arg_keys"] = JsonSerializer.SerializeToNode(argKeys, _jsonOptions), ["arg_lengths"] = argsObject, }; + toolDisplay.AddMetadata(evt, "tool"); DeferFrameLog(() => WriteMcpLogLine(evt.ToJsonString(_jsonOptions))); } @@ -2534,9 +2533,10 @@ private void TryEmitAudit(string toolName, JsonNode? id, JsonNode? args, JsonNod var (errorCode, observedErrorType) = ExtractErrorCode(response); var resultCount = ExtractResultCount(response); var (argKeys, argLengths, argValuesEcho) = SanitizeArgs(args, _auditLog.IncludeValues); + var toolDisplay = BoundToolNameForDisplay(toolName); var evt = new AuditLogSink.AuditEvent( Timestamp: startedAt, - Tool: toolName, + Tool: toolDisplay.Text, CallerName: _clientName, CallerVersion: _clientVersion, RequestId: SerializeRequestId(id), @@ -2703,7 +2703,7 @@ internal static string BuildResponseWriteErrorLog(string detail) => $"[cdidx-mcp] Error writing response: {detail}. The request was handled but the client connection may already be closed."; internal static string BuildToolErrorLog(string toolName, string detail) => - $"[cdidx-mcp] Tool error ({toolName}): {detail}. Fix the tool arguments, refresh the index if needed, then retry."; + $"[cdidx-mcp] Tool error ({BoundToolNameForDisplay(toolName).Text}): {detail}. Fix the tool arguments, refresh the index if needed, then retry."; // Stderr log emitted when the rate limiter denies a tool call. Mirrors the JSON-RPC // `-32000` payload (tool + caller + retry_after_ms) so operators tailing the MCP log @@ -2711,7 +2711,7 @@ internal static string BuildToolErrorLog(string toolName, string detail) => // レート制限で拒否されたツール呼び出しを stderr に記録する。配線上の JSON-RPC `-32000` // ペイロードと内容を揃え、運用側がログ追跡から状況把握できるようにする（#1560）。 internal static string BuildRateLimitedLog(string toolName, string caller, long retryAfterMs) => - $"[cdidx-mcp] Rate limit exceeded: tool='{toolName}', caller='{caller}', retry_after_ms={retryAfterMs}. Increase {RateLimiterOptions.RpsEnvVar} / {RateLimiterOptions.BurstEnvVar} on the server, or back off and retry."; + $"[cdidx-mcp] Rate limit exceeded: tool='{BoundToolNameForDisplay(toolName).Text}', caller='{caller}', retry_after_ms={retryAfterMs}. Increase {RateLimiterOptions.RpsEnvVar} / {RateLimiterOptions.BurstEnvVar} on the server, or back off and retry."; internal static string BuildCallerSwapRejectionLog(string current, string attempted) => $"[cdidx-mcp] Ignoring re-initialize with new clientInfo identity '{attempted}': retaining original caller '{current}' so rate-limit buckets cannot be reset mid-session."; @@ -2757,11 +2757,12 @@ internal static string FormatDbPathForLog(string dbPath) // #1530 で封じた ex.Message 漏れを再現させずに失敗詳細をクライアントへ届ける。 internal static string BuildSanitizedToolErrorMessage(string toolName, Exception ex) { + var toolDisplay = BoundToolNameForDisplay(toolName).Text; if (!IsUnsafeDebugEnabled()) - return $"Tool '{toolName}' failed. See cdidx server stderr for details."; + return $"Tool '{toolDisplay}' failed. See cdidx server stderr for details."; if (ex is CodeIndexException codeIndexEx) - return $"Error executing {toolName} ({ex.GetType().Name}) [{codeIndexEx.Code}/{codeIndexEx.Category}]{BuildPathFragment(codeIndexEx)}{BuildHintFragment(codeIndexEx)}. See cdidx server stderr for details."; - return $"Error executing {toolName} ({ex.GetType().Name}). See cdidx server stderr for details."; + return $"Error executing {toolDisplay} ({ex.GetType().Name}) [{codeIndexEx.Code}/{codeIndexEx.Category}]{BuildPathFragment(codeIndexEx)}{BuildHintFragment(codeIndexEx)}. See cdidx server stderr for details."; + return $"Error executing {toolDisplay} ({ex.GetType().Name}). See cdidx server stderr for details."; } // Wire-safe error body for the JSON-RPC loop catch-all. Same rationale as @@ -2966,6 +2967,49 @@ private static JsonObject CreateErrorResponse(JsonNode? id, int code, string mes string category, string suggestion, bool retrySafe, JsonObject? extraData = null) => CreateErrorResponse(id is not null, id, code, message, category, suggestion, retrySafe, extraData); + private static BoundedMcpText BoundToolNameForDisplay(string toolName) + => McpBoundedText.ForDisplay(toolName, McpBoundedText.MaxToolNameChars); + + private static void AddToolDisplayData(JsonObject target, string? toolName) + { + if (toolName is null) + { + target["tool"] = null; + return; + } + + var display = BoundToolNameForDisplay(toolName); + target["tool"] = display.Text; + display.AddMetadata(target, "tool"); + } + + internal static string BuildUnknownToolMessage(string toolName) + => $"Unknown tool: {BoundToolNameForDisplay(toolName).Text}"; + + private static JsonObject BuildUnknownToolData(string toolName) + { + var data = new JsonObject(); + AddToolDisplayData(data, toolName); + return data; + } + + private static JsonObject BuildToolExceptionData(string toolName, string exceptionType) + { + var data = new JsonObject + { + ["exception_type"] = exceptionType, + }; + AddToolDisplayData(data, toolName); + return data; + } + + private static JsonObject CreateUnknownToolErrorResponse(bool hasId, JsonNode? id, string toolName) + => CreateErrorResponse(hasId: hasId, id: id, code: -32602, message: BuildUnknownToolMessage(toolName), + category: McpErrorEnvelope.CategoryToolUnknown, + suggestion: "Call tools/list to enumerate the available tool names for this server. Tool name match is case-sensitive.", + retrySafe: false, + extraData: BuildUnknownToolData(toolName)); + // Issue #1581: every MCP error response carries a structured `data` envelope // (`category` / `suggestion` / `retry_safe`) so clients can branch on a stable // category instead of parsing the human-readable `message`. Category-specific diff --git a/src/CodeIndex/Mcp/McpToolHandlers.cs b/src/CodeIndex/Mcp/McpToolHandlers.cs index ae7b1e7ec5..836a18f64c 100644 --- a/src/CodeIndex/Mcp/McpToolHandlers.cs +++ b/src/CodeIndex/Mcp/McpToolHandlers.cs @@ -2511,13 +2511,14 @@ bool TryAppendResult(JsonObject entry, string? toolName, JsonNode? toolArgs, int { truncated = true; cascadeStartedAtIndex ??= requestIndex; - truncatedQueries.Add(new JsonObject + var truncatedEntry = new JsonObject { ["request_index"] = requestIndex, - ["tool"] = toolName, ["args_summary"] = BuildArgsSummary(toolArgs), ["reason"] = "response_byte_limit_exceeded", - }); + }; + AddToolDisplayData(truncatedEntry, toolName); + truncatedQueries.Add(truncatedEntry); return false; } @@ -2533,13 +2534,13 @@ void AppendSlotError(int requestIndex, string? toolName, JsonNode? toolArgs, Sto var entry = new JsonObject { ["request_index"] = requestIndex, - ["tool"] = toolName, ["ok"] = false, ["correlation_id"] = CurrentCorrelationContext.Value?.CorrelationId, ["args_summary"] = BuildArgsSummary(toolArgs), ["elapsed_ms"] = slotStopwatch.ElapsedMilliseconds, ["error"] = errorMessage, }; + AddToolDisplayData(entry, toolName); if (code.HasValue) entry["code"] = code.Value; // #1581: batch_query slot errors also carry the canonical envelope so clients @@ -2630,6 +2631,14 @@ void AppendRateLimitedSlot(int requestIndex, string? toolName, JsonNode? toolArg retrySafe: false); continue; } + if (toolName.Length > McpBoundedText.MaxToolNameChars) + { + AppendSlotError(requestIndex, toolName, toolArgs, slotStopwatch, BuildUnknownToolMessage(toolName), + category: McpErrorEnvelope.CategoryToolUnknown, + suggestion: "Call tools/list to see the tool catalog. Slot tool names are case-sensitive.", + retrySafe: false); + continue; + } if (ValidateToolArguments(toolName, toolArgs) is JsonObject argumentError) { @@ -2743,7 +2752,7 @@ void AppendRateLimitedSlot(int requestIndex, string? toolName, JsonNode? toolArg if (response == null) { - AppendSlotError(requestIndex, toolName, toolArgs, slotStopwatch, $"Unknown tool: {toolName}", + AppendSlotError(requestIndex, toolName, toolArgs, slotStopwatch, BuildUnknownToolMessage(toolName), category: McpErrorEnvelope.CategoryToolUnknown, suggestion: "Call tools/list to see the tool catalog. Slot tool names are case-sensitive.", retrySafe: false); @@ -2781,13 +2790,13 @@ void AppendRateLimitedSlot(int requestIndex, string? toolName, JsonNode? toolArg var entry = new JsonObject { ["request_index"] = requestIndex, - ["tool"] = toolName, ["ok"] = true, ["correlation_id"] = CurrentCorrelationContext.Value?.CorrelationId, ["args_summary"] = BuildArgsSummary(toolArgs), ["elapsed_ms"] = slotStopwatch.ElapsedMilliseconds, ["result"] = structured?.DeepClone(), }; + AddToolDisplayData(entry, toolName); TryAppendResult(entry, toolName, toolArgs, requestIndex, successfulSlot: true); successCount++; } diff --git a/tests/CodeIndex.Tests/McpAuditLogTests.cs b/tests/CodeIndex.Tests/McpAuditLogTests.cs index 43485cb3a1..c4c45ecd5e 100644 --- a/tests/CodeIndex.Tests/McpAuditLogTests.cs +++ b/tests/CodeIndex.Tests/McpAuditLogTests.cs @@ -154,6 +154,38 @@ public void ToolsCall_UnknownTool_EmitsAuditRecordWithInvalidParamsCode() Assert.Equal("x", record.GetProperty("arg_keys")[0].GetString()); } + [Fact] + public void ToolsCall_UnknownTool_TruncatesAuditToolName_Issue3118() + { + using var sink = new AuditLogSink(_auditPath, AuditLogSink.DefaultMaxBytes, includeValues: false); + using var server = CreateServer(sink); + var toolName = new string('u', McpBoundedText.MaxToolNameChars + 25); + var display = McpBoundedText.ForDisplay(toolName, McpBoundedText.MaxToolNameChars); + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 1, + ["method"] = "tools/call", + ["params"] = new JsonObject + { + ["name"] = toolName, + ["arguments"] = new JsonObject + { + ["x"] = 1, + }, + }, + }; + + var response = server.HandleMessage(request)!; + + Assert.Equal(-32602, response["error"]!["code"]!.GetValue()); + var rawLog = File.ReadAllText(_auditPath); + Assert.DoesNotContain(toolName, rawLog, StringComparison.Ordinal); + var record = ReadOnlyRecord(); + Assert.Equal(display.Text, record.GetProperty("tool").GetString()); + Assert.Equal(-32602, record.GetProperty("error_code").GetInt32()); + } + [Fact] public void ToolsCall_IncludeValues_EchoesArgValuesIntoRecord() { diff --git a/tests/CodeIndex.Tests/McpServerTests.cs b/tests/CodeIndex.Tests/McpServerTests.cs index 4ff8d3a9a6..429673613d 100644 --- a/tests/CodeIndex.Tests/McpServerTests.cs +++ b/tests/CodeIndex.Tests/McpServerTests.cs @@ -528,6 +528,63 @@ await Task.Run(() => Assert.False(string.IsNullOrWhiteSpace(correlationId.GetString())); } + [Fact] + public async Task ProcessLineAsync_UnknownToolName_TruncatesTelemetry_Issue3118() + { + using var writer = new StringWriter(); + using var error = new StringWriter(); + var toolName = new string('l', McpBoundedText.MaxToolNameChars + 25); + var display = McpBoundedText.ForDisplay(toolName, McpBoundedText.MaxToolNameChars); + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 123, + ["method"] = "tools/call", + ["params"] = new JsonObject + { + ["name"] = toolName, + ["arguments"] = new JsonObject + { + ["x"] = 1, + }, + }, + }; + + await Task.Run(() => + { + lock (TestConsoleLock.Gate) + { + var previousError = Console.Error; + try + { + Console.SetError(error); +#pragma warning disable xUnit1031 + _server.ProcessLineAsync(request.ToJsonString(), writer).GetAwaiter().GetResult(); +#pragma warning restore xUnit1031 + } + finally + { + Console.SetError(previousError); + } + } + }); + + Assert.DoesNotContain(toolName, writer.ToString(), StringComparison.Ordinal); + Assert.DoesNotContain(toolName, error.ToString(), StringComparison.Ordinal); + var line = error.ToString() + .Split('\n', StringSplitOptions.RemoveEmptyEntries) + .Single(l => l.Contains("\"event\":\"mcp.tool.invocation\"", StringComparison.Ordinal)); + var jsonStart = line.IndexOf('{'); + using var document = JsonDocument.Parse(line[jsonStart..]); + var root = document.RootElement; + Assert.Equal("mcp.tool.invocation", root.GetProperty("event").GetString()); + Assert.Equal(display.Text, root.GetProperty("tool").GetString()); + Assert.Equal(toolName.Length, root.GetProperty("tool_length").GetInt32()); + Assert.True(root.GetProperty("tool_truncated").GetBoolean()); + Assert.Equal("error", root.GetProperty("status").GetString()); + Assert.Equal(-32602, root.GetProperty("error_code").GetInt32()); + } + [Fact] public async Task ProcessLineAsync_FallbackErrorIncludesCorrelationData() { @@ -6968,6 +7025,50 @@ public void ToolsCall_BatchQuery_CountsFailuresInEnvelope_Issue1537() Assert.Contains("1 succeeded, 2 failed", text); } + [Fact] + public void ToolsCall_BatchQuery_UnknownToolName_TruncatesDisplay_Issue3118() + { + var toolName = new string('b', McpBoundedText.MaxToolNameChars + 25); + var display = McpBoundedText.ForDisplay(toolName, McpBoundedText.MaxToolNameChars); + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 1, + ["method"] = "tools/call", + ["params"] = new JsonObject + { + ["name"] = "batch_query", + ["arguments"] = new JsonObject + { + ["queries"] = new JsonArray + { + new JsonObject + { + ["tool"] = toolName, + ["arguments"] = new JsonObject + { + ["x"] = 1, + }, + }, + }, + }, + }, + }; + + var response = _server.HandleMessage(request)!; + + Assert.DoesNotContain(toolName, response.ToJsonString(), StringComparison.Ordinal); + var result = response["result"]!; + var structured = result["structuredContent"]!; + Assert.True(structured["partial_failure"]!.GetValue()); + var slot = Assert.Single(structured["results"]!.AsArray())!; + Assert.False(slot["ok"]!.GetValue()); + Assert.Equal(display.Text, slot["tool"]!.GetValue()); + Assert.Equal(toolName.Length, slot["tool_length"]!.GetValue()); + Assert.True(slot["tool_truncated"]!.GetValue()); + Assert.Equal($"Unknown tool: {display.Text}", slot["error"]!.GetValue()); + } + [Fact] public void ToolsCall_BatchQuery_SanitizesSlotExceptionMessage_Issue2849() { @@ -7335,6 +7436,37 @@ public void ToolsCall_UnknownArgumentName_TruncatesDisplay_Issue3117() Assert.True(structured["unknown_argument_truncated"]!.GetValue()); } + [Fact] + public void ToolsCall_UnknownToolName_TruncatesDisplay_Issue3118() + { + var toolName = new string('t', McpBoundedText.MaxToolNameChars + 25); + var display = McpBoundedText.ForDisplay(toolName, McpBoundedText.MaxToolNameChars); + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 1, + ["method"] = "tools/call", + ["params"] = new JsonObject + { + ["name"] = toolName, + ["arguments"] = new JsonObject + { + ["x"] = 1, + }, + }, + }; + + var response = _server.HandleMessage(request)!; + + Assert.Equal(-32602, response["error"]!["code"]!.GetValue()); + Assert.DoesNotContain(toolName, response.ToJsonString(), StringComparison.Ordinal); + Assert.Equal($"Unknown tool: {display.Text}", response["error"]!["message"]!.GetValue()); + var data = response["error"]!["data"]!; + Assert.Equal(display.Text, data["tool"]!.GetValue()); + Assert.Equal(toolName.Length, data["tool_length"]!.GetValue()); + Assert.True(data["tool_truncated"]!.GetValue()); + } + [Fact] public void SanitizeArgs_TruncatesArgumentKeysForAuditAndTelemetry_Issue3117() { From 02a7820ed03855643fde401ef03585209feea9db Mon Sep 17 00:00:00 2001 From: Widthdom Date: Thu, 4 Jun 2026 22:48:54 +0900 Subject: [PATCH 04/11] Bound MCP protocol version diagnostics (#3119) --- changelog.d/unreleased/3119.security.md | 17 +++++ src/CodeIndex/Mcp/McpBoundedText.cs | 1 + src/CodeIndex/Mcp/McpServer.cs | 26 ++++++-- tests/CodeIndex.Tests/McpServerTests.cs | 84 +++++++++++++++++++++++++ 4 files changed, 122 insertions(+), 6 deletions(-) create mode 100644 changelog.d/unreleased/3119.security.md diff --git a/changelog.d/unreleased/3119.security.md b/changelog.d/unreleased/3119.security.md new file mode 100644 index 0000000000..8a05e1eec7 --- /dev/null +++ b/changelog.d/unreleased/3119.security.md @@ -0,0 +1,17 @@ +--- +category: security +issues: + - 3119 +affected: + - src/CodeIndex/Mcp/McpBoundedText.cs + - src/CodeIndex/Mcp/McpServer.cs + - tests/CodeIndex.Tests/McpServerTests.cs +--- + +## English + +- **MCP unsupported protocol diagnostics truncate requested versions (#3119)** — unsupported `protocolVersion` values are bounded during negotiation before appearing in initialize errors, structured data, or stderr logs, with original length metadata when truncation occurs. + +## 日本語 + +- **MCP の非対応 protocol 診断で要求versionを切り詰めるようになりました (#3119)** — 非対応の `protocolVersion` は交渉時に bounded display 化し、initialize error、structured data、stderr log には切り詰めた値と長さmetadataだけを返します。 diff --git a/src/CodeIndex/Mcp/McpBoundedText.cs b/src/CodeIndex/Mcp/McpBoundedText.cs index e54bb42a65..45f1b9dbf8 100644 --- a/src/CodeIndex/Mcp/McpBoundedText.cs +++ b/src/CodeIndex/Mcp/McpBoundedText.cs @@ -20,6 +20,7 @@ internal static class McpBoundedText internal const int MaxScalarArgumentChars = 512; internal const int MaxDiagnosticDisplayChars = 128; internal const int MaxToolNameChars = 128; + internal const int MaxProtocolVersionChars = 128; internal static BoundedMcpText ForDisplay(string value, int maxChars = MaxDiagnosticDisplayChars) { diff --git a/src/CodeIndex/Mcp/McpServer.cs b/src/CodeIndex/Mcp/McpServer.cs index f5a2f3a96e..12fc392240 100644 --- a/src/CodeIndex/Mcp/McpServer.cs +++ b/src/CodeIndex/Mcp/McpServer.cs @@ -2105,7 +2105,7 @@ internal static string ResolveCallerIdentity(JsonNode? initializeParams) return version == null ? name : $"{name}/{version}"; } - internal static string? NegotiateProtocolVersion(JsonNode? initializeParams, out string? requestedVersion) + internal static string? NegotiateProtocolVersion(JsonNode? initializeParams, out BoundedMcpText? requestedVersion) { requestedVersion = null; if (initializeParams is JsonObject obj @@ -2114,7 +2114,7 @@ internal static string ResolveCallerIdentity(JsonNode? initializeParams) && value.TryGetValue(out var versionString) && !string.IsNullOrWhiteSpace(versionString)) { - requestedVersion = versionString; + requestedVersion = BoundProtocolVersionForDisplay(versionString); foreach (var supported in SupportedProtocolVersions) { if (string.Equals(supported, versionString, StringComparison.Ordinal)) @@ -2129,7 +2129,7 @@ internal static string ResolveCallerIdentity(JsonNode? initializeParams) return ProtocolVersion; } - private static JsonObject CreateUnsupportedProtocolError(JsonNode? id, string? requestedVersion) + private static JsonObject CreateUnsupportedProtocolError(JsonNode? id, BoundedMcpText? requestedVersion) { var supportedArray = new JsonArray(); foreach (var supported in SupportedProtocolVersions) @@ -2146,7 +2146,10 @@ private static JsonObject CreateUnsupportedProtocolError(JsonNode? id, string? r ["supportedVersions"] = supportedArray }; if (requestedVersion != null) - extra["requestedVersion"] = requestedVersion; + { + extra["requestedVersion"] = requestedVersion.Value.Text; + requestedVersion.Value.AddMetadata(extra, "requestedVersion"); + } var data = McpErrorEnvelope.BuildData( McpErrorEnvelope.CategoryInvalidArgument, @@ -2170,19 +2173,30 @@ private static JsonObject CreateUnsupportedProtocolError(JsonNode? id, string? r } internal static string BuildUnsupportedProtocolMessage(string? requestedVersion) + => BuildUnsupportedProtocolMessage(BoundProtocolVersionForDisplay(requestedVersion)); + + private static string BuildUnsupportedProtocolMessage(BoundedMcpText? requestedVersion) { var supported = string.Join(", ", SupportedProtocolVersions); - var requested = string.IsNullOrEmpty(requestedVersion) ? "(unspecified)" : requestedVersion; + var requested = requestedVersion?.Text ?? "(unspecified)"; return $"Unsupported MCP protocolVersion '{requested}'. Server supports: {supported}."; } internal static string BuildUnsupportedProtocolLog(string? requestedVersion) + => BuildUnsupportedProtocolLog(BoundProtocolVersionForDisplay(requestedVersion)); + + private static string BuildUnsupportedProtocolLog(BoundedMcpText? requestedVersion) { var supported = string.Join(", ", SupportedProtocolVersions); - var requested = string.IsNullOrEmpty(requestedVersion) ? "(unspecified)" : requestedVersion; + var requested = requestedVersion?.Text ?? "(unspecified)"; return $"[cdidx-mcp] Rejecting initialize: client requested protocolVersion '{requested}', server supports {supported}. Upgrade the server or pin a supported version on the client."; } + private static BoundedMcpText? BoundProtocolVersionForDisplay(string? requestedVersion) + => string.IsNullOrEmpty(requestedVersion) + ? null + : McpBoundedText.ForDisplay(requestedVersion, McpBoundedText.MaxProtocolVersionChars); + /// /// Build a structured `-32000` JSON-RPC error for a rate-limited tool call. Surfacing /// the limit category in `error.data.error_category` (alongside `tool`, `caller`, and diff --git a/tests/CodeIndex.Tests/McpServerTests.cs b/tests/CodeIndex.Tests/McpServerTests.cs index 429673613d..1994a18142 100644 --- a/tests/CodeIndex.Tests/McpServerTests.cs +++ b/tests/CodeIndex.Tests/McpServerTests.cs @@ -1091,6 +1091,52 @@ public void BuildUnsupportedProtocolLog_IsActionable() Assert.Contains("Upgrade the server or pin a supported version", log); } + [Fact] + public void NegotiateProtocolVersion_TruncatesUnsupportedRequestedVersion_Issue3119() + { + var requested = new string('v', McpBoundedText.MaxProtocolVersionChars + 25); + var display = McpBoundedText.ForDisplay(requested, McpBoundedText.MaxProtocolVersionChars); + var initializeParams = new JsonObject + { + ["protocolVersion"] = requested, + }; + + var negotiated = McpServer.NegotiateProtocolVersion(initializeParams, out var requestedVersion); + + Assert.Null(negotiated); + Assert.NotNull(requestedVersion); + Assert.Equal(display.Text, requestedVersion.Value.Text); + Assert.Equal(requested.Length, requestedVersion.Value.OriginalLength); + Assert.True(requestedVersion.Value.Truncated); + } + + [Fact] + public void Initialize_UnsupportedProtocolVersion_TruncatesDiagnostics_Issue3119() + { + var requested = new string('p', McpBoundedText.MaxProtocolVersionChars + 25); + var display = McpBoundedText.ForDisplay(requested, McpBoundedText.MaxProtocolVersionChars); + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 1, + ["method"] = "initialize", + ["params"] = new JsonObject + { + ["protocolVersion"] = requested, + }, + }; + + var response = _server.HandleMessage(request)!; + + Assert.Equal(-32602, response["error"]!["code"]!.GetValue()); + Assert.DoesNotContain(requested, response.ToJsonString(), StringComparison.Ordinal); + Assert.Contains(display.Text, response["error"]!["message"]!.GetValue()); + var data = response["error"]!["data"]!; + Assert.Equal(display.Text, data["requestedVersion"]!.GetValue()); + Assert.Equal(requested.Length, data["requestedVersion_length"]!.GetValue()); + Assert.True(data["requestedVersion_truncated"]!.GetValue()); + } + [Fact] public void Initialize_NullId_PreservesNullResponseId() { @@ -1861,6 +1907,44 @@ await server.ProcessLineAsync( Assert.Contains("Rejecting initialize", error.ToString()); } + [Fact] + public async Task ProcessLineAsync_UnsupportedProtocol_TruncatesErrorLog_Issue3119() + { + using var server = new McpServer(_dbPath, ConsoleUi.LoadVersion()); + using var writer = new StringWriter(); + using var error = new StringWriter(); + var requested = new string('q', McpBoundedText.MaxProtocolVersionChars + 25); + var display = McpBoundedText.ForDisplay(requested, McpBoundedText.MaxProtocolVersionChars); + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 1, + ["method"] = "initialize", + ["params"] = new JsonObject + { + ["protocolVersion"] = requested, + }, + }; + var previousError = Console.Error; + Console.SetError(error); + try + { + await server.ProcessLineAsync( + request.ToJsonString(), + new AssertingTextWriter(writer, () => Assert.Equal(string.Empty, error.ToString()))); + } + finally + { + Console.SetError(previousError); + } + + Assert.DoesNotContain(requested, writer.ToString(), StringComparison.Ordinal); + Assert.DoesNotContain(requested, error.ToString(), StringComparison.Ordinal); + Assert.Contains(display.Text, writer.ToString()); + Assert.Contains(display.Text, error.ToString()); + Assert.Contains("Rejecting initialize", error.ToString()); + } + [Fact] public async Task ProcessLineAsync_AuthFailure_WritesResponseBeforeErrorLog() { From 687cf3c073d39e0877c937c3d4ebcf7f4d5a814a Mon Sep 17 00:00:00 2001 From: Widthdom Date: Thu, 4 Jun 2026 23:04:51 +0900 Subject: [PATCH 05/11] Bound MCP client info diagnostics (#3120) --- changelog.d/unreleased/3120.security.md | 20 +++++++ src/CodeIndex/Mcp/AuditLogSink.cs | 14 ++++- src/CodeIndex/Mcp/McpBoundedText.cs | 2 + src/CodeIndex/Mcp/McpServer.cs | 54 ++++++++++------- src/CodeIndex/Mcp/McpToolHandlers.cs | 10 +++- tests/CodeIndex.Tests/McpAuditLogTests.cs | 40 +++++++++++++ tests/CodeIndex.Tests/McpServerTests.cs | 73 +++++++++++++++++++++++ 7 files changed, 189 insertions(+), 24 deletions(-) create mode 100644 changelog.d/unreleased/3120.security.md diff --git a/changelog.d/unreleased/3120.security.md b/changelog.d/unreleased/3120.security.md new file mode 100644 index 0000000000..4ebf7491cd --- /dev/null +++ b/changelog.d/unreleased/3120.security.md @@ -0,0 +1,20 @@ +--- +category: security +issues: + - 3120 +affected: + - src/CodeIndex/Mcp/AuditLogSink.cs + - src/CodeIndex/Mcp/McpBoundedText.cs + - src/CodeIndex/Mcp/McpServer.cs + - src/CodeIndex/Mcp/McpToolHandlers.cs + - tests/CodeIndex.Tests/McpAuditLogTests.cs + - tests/CodeIndex.Tests/McpServerTests.cs +--- + +## English + +- **MCP clientInfo fields are bounded before reuse (#3120)** — `clientInfo.name` and `clientInfo.version` are captured as bounded display values before status output, caller identity, rate-limit diagnostics, caller-swap logs, and audit records reuse them, with truncation metadata where values are shortened. + +## 日本語 + +- **MCP の clientInfo フィールドを再利用前に制限するようになりました (#3120)** — `clientInfo.name` と `clientInfo.version` は status 出力、caller identity、rate-limit 診断、caller-swap log、audit record で再利用する前に bounded display 化し、切り詰め時は metadata も出力します。 diff --git a/src/CodeIndex/Mcp/AuditLogSink.cs b/src/CodeIndex/Mcp/AuditLogSink.cs index 6065ceb497..869299a2eb 100644 --- a/src/CodeIndex/Mcp/AuditLogSink.cs +++ b/src/CodeIndex/Mcp/AuditLogSink.cs @@ -214,8 +214,16 @@ internal static string SerializeEvent(AuditEvent evt, bool includeValues) jw.WriteString("tool", evt.Tool); if (evt.CallerName is { } caller) jw.WriteString("caller", caller); + if (evt.CallerNameLength is { } callerLength) + jw.WriteNumber("caller_length", callerLength); + if (evt.CallerNameTruncated) + jw.WriteBoolean("caller_truncated", true); if (evt.CallerVersion is { } callerVersion) jw.WriteString("caller_version", callerVersion); + if (evt.CallerVersionLength is { } callerVersionLength) + jw.WriteNumber("caller_version_length", callerVersionLength); + if (evt.CallerVersionTruncated) + jw.WriteBoolean("caller_version_truncated", true); if (evt.RequestId is { } reqId) jw.WriteString("request_id", reqId); @@ -278,5 +286,9 @@ internal sealed record AuditEvent( int? ResultCount, double ElapsedMs, int ErrorCode, - string? ErrorType); + string? ErrorType, + int? CallerNameLength = null, + bool CallerNameTruncated = false, + int? CallerVersionLength = null, + bool CallerVersionTruncated = false); } diff --git a/src/CodeIndex/Mcp/McpBoundedText.cs b/src/CodeIndex/Mcp/McpBoundedText.cs index 45f1b9dbf8..161cdfc893 100644 --- a/src/CodeIndex/Mcp/McpBoundedText.cs +++ b/src/CodeIndex/Mcp/McpBoundedText.cs @@ -21,6 +21,8 @@ internal static class McpBoundedText internal const int MaxDiagnosticDisplayChars = 128; internal const int MaxToolNameChars = 128; internal const int MaxProtocolVersionChars = 128; + internal const int MaxClientInfoChars = 128; + internal const int MaxClientIdentityChars = (MaxClientInfoChars * 2) + 1; internal static BoundedMcpText ForDisplay(string value, int maxChars = MaxDiagnosticDisplayChars) { diff --git a/src/CodeIndex/Mcp/McpServer.cs b/src/CodeIndex/Mcp/McpServer.cs index 12fc392240..115789de63 100644 --- a/src/CodeIndex/Mcp/McpServer.cs +++ b/src/CodeIndex/Mcp/McpServer.cs @@ -109,6 +109,8 @@ public partial class McpServer : IDisposable // にする。`initialize` 毎に上書きすることで再接続時に caller identity が追随する。 private string? _clientName; private string? _clientVersion; + private BoundedMcpText? _clientNameDisplay; + private BoundedMcpText? _clientVersionDisplay; private JsonNode? _clientCapabilities; private JsonArray _clientRoots = []; private string _mcpLogLevel = "info"; @@ -1801,13 +1803,17 @@ private void CaptureClientInfo(JsonNode? initializeParams) // 再接続が前回のクライアント名/version を引き継がないようにするため。 _clientName = null; _clientVersion = null; + _clientNameDisplay = null; + _clientVersionDisplay = null; if (initializeParams is not JsonObject obj) return; _clientRootsStale = true; if (obj["clientInfo"] is not JsonObject info) return; - _clientName = TryReadStringMember(info, "name"); - _clientVersion = TryReadStringMember(info, "version"); + _clientNameDisplay = TryReadBoundedClientInfoMember(info, "name"); + _clientVersionDisplay = TryReadBoundedClientInfoMember(info, "version"); + _clientName = _clientNameDisplay?.Text; + _clientVersion = _clientVersionDisplay?.Text; } private void CaptureClientSession(JsonNode? initializeParams) @@ -1853,10 +1859,16 @@ private void CaptureClientSession(JsonNode? initializeParams) if (!obj.TryGetPropertyValue(key, out var node)) return null; if (node is JsonValue value && value.TryGetValue(out var s) && !string.IsNullOrWhiteSpace(s)) - return s; + return s.Trim(); return null; } + private static BoundedMcpText? TryReadBoundedClientInfoMember(JsonObject obj, string key) + { + var value = TryReadStringMember(obj, key); + return value is null ? null : BoundClientInfoForDisplay(value); + } + private JsonNode HandleResourcesList(JsonNode? id, JsonNode? listParams) { const int pageSize = 200; @@ -2086,22 +2098,10 @@ internal static string ResolveCallerIdentity(JsonNode? initializeParams) if (obj["clientInfo"] is not JsonObject clientInfo) return "unknown"; - string? Read(string key) - { - if (clientInfo.TryGetPropertyValue(key, out var node) - && node is JsonValue value - && value.TryGetValue(out var s) - && !string.IsNullOrWhiteSpace(s)) - { - return s.Trim(); - } - return null; - } - - var name = Read("name"); + var name = TryReadBoundedClientInfoMember(clientInfo, "name")?.Text; if (name == null) return "unknown"; - var version = Read("version"); + var version = TryReadBoundedClientInfoMember(clientInfo, "version")?.Text; return version == null ? name : $"{name}/{version}"; } @@ -2197,6 +2197,12 @@ private static string BuildUnsupportedProtocolLog(BoundedMcpText? requestedVersi ? null : McpBoundedText.ForDisplay(requestedVersion, McpBoundedText.MaxProtocolVersionChars); + private static BoundedMcpText BoundClientInfoForDisplay(string value) + => McpBoundedText.ForDisplay(value, McpBoundedText.MaxClientInfoChars); + + private static BoundedMcpText BoundClientIdentityForDisplay(string value) + => McpBoundedText.ForDisplay(value, McpBoundedText.MaxClientIdentityChars); + /// /// Build a structured `-32000` JSON-RPC error for a rate-limited tool call. Surfacing /// the limit category in `error.data.error_category` (alongside `tool`, `caller`, and @@ -2208,6 +2214,7 @@ private static string BuildUnsupportedProtocolLog(BoundedMcpText? requestedVersi /// internal static JsonObject CreateRateLimitedErrorResponse(JsonNode? id, string tool, string caller, long retryAfterMs) { + var callerDisplay = BoundClientIdentityForDisplay(caller); // #1560 contract preserved: `error_category`, `tool`, `caller`, `retry_after_ms`. // #1581 adds the canonical envelope (`category`, `suggestion`, `retry_safe`) alongside. // #1560 の契約（`error_category`, `tool`, `caller`, `retry_after_ms`）を維持しつつ、 @@ -2216,9 +2223,10 @@ internal static JsonObject CreateRateLimitedErrorResponse(JsonNode? id, string t { ["error_category"] = "rate_limited", ["tool"] = tool, - ["caller"] = caller, + ["caller"] = callerDisplay.Text, ["retry_after_ms"] = retryAfterMs, }; + callerDisplay.AddMetadata(extraData, "caller"); var data = McpErrorEnvelope.BuildData( category: McpErrorEnvelope.CategoryRateLimited, suggestion: $"Back off for at least {retryAfterMs} ms before retrying this tool, or raise {RateLimiterOptions.RpsEnvVar} / {RateLimiterOptions.BurstEnvVar} on the server.", @@ -2560,7 +2568,11 @@ private void TryEmitAudit(string toolName, JsonNode? id, JsonNode? args, JsonNod ResultCount: resultCount, ElapsedMs: elapsedMs, ErrorCode: errorCode, - ErrorType: errorType ?? observedErrorType); + ErrorType: errorType ?? observedErrorType, + CallerNameLength: _clientNameDisplay?.Truncated == true ? _clientNameDisplay.Value.OriginalLength : null, + CallerNameTruncated: _clientNameDisplay?.Truncated == true, + CallerVersionLength: _clientVersionDisplay?.Truncated == true ? _clientVersionDisplay.Value.OriginalLength : null, + CallerVersionTruncated: _clientVersionDisplay?.Truncated == true); _auditLog.Record(evt); } catch @@ -2725,10 +2737,10 @@ internal static string BuildToolErrorLog(string toolName, string detail) => // レート制限で拒否されたツール呼び出しを stderr に記録する。配線上の JSON-RPC `-32000` // ペイロードと内容を揃え、運用側がログ追跡から状況把握できるようにする（#1560）。 internal static string BuildRateLimitedLog(string toolName, string caller, long retryAfterMs) => - $"[cdidx-mcp] Rate limit exceeded: tool='{BoundToolNameForDisplay(toolName).Text}', caller='{caller}', retry_after_ms={retryAfterMs}. Increase {RateLimiterOptions.RpsEnvVar} / {RateLimiterOptions.BurstEnvVar} on the server, or back off and retry."; + $"[cdidx-mcp] Rate limit exceeded: tool='{BoundToolNameForDisplay(toolName).Text}', caller='{BoundClientIdentityForDisplay(caller).Text}', retry_after_ms={retryAfterMs}. Increase {RateLimiterOptions.RpsEnvVar} / {RateLimiterOptions.BurstEnvVar} on the server, or back off and retry."; internal static string BuildCallerSwapRejectionLog(string current, string attempted) => - $"[cdidx-mcp] Ignoring re-initialize with new clientInfo identity '{attempted}': retaining original caller '{current}' so rate-limit buckets cannot be reset mid-session."; + $"[cdidx-mcp] Ignoring re-initialize with new clientInfo identity '{BoundClientIdentityForDisplay(attempted).Text}': retaining original caller '{BoundClientIdentityForDisplay(current).Text}' so rate-limit buckets cannot be reset mid-session."; internal static string BuildUnknownNotificationLog(string method) => $"[cdidx-mcp] Ignoring unknown notification: {method}"; diff --git a/src/CodeIndex/Mcp/McpToolHandlers.cs b/src/CodeIndex/Mcp/McpToolHandlers.cs index 836a18f64c..336c253638 100644 --- a/src/CodeIndex/Mcp/McpToolHandlers.cs +++ b/src/CodeIndex/Mcp/McpToolHandlers.cs @@ -2148,10 +2148,16 @@ private JsonObject BuildMcpSessionStatus() if (_clientName is not null || _clientVersion is not null) { var clientInfo = new JsonObject(); - if (_clientName is not null) + if (_clientNameDisplay is not null) + { clientInfo["name"] = _clientName; - if (_clientVersion is not null) + _clientNameDisplay.Value.AddMetadata(clientInfo, "name"); + } + if (_clientVersionDisplay is not null) + { clientInfo["version"] = _clientVersion; + _clientVersionDisplay.Value.AddMetadata(clientInfo, "version"); + } session["client_info"] = clientInfo; } if (_clientCapabilities is not null) diff --git a/tests/CodeIndex.Tests/McpAuditLogTests.cs b/tests/CodeIndex.Tests/McpAuditLogTests.cs index c4c45ecd5e..e31c3c55e7 100644 --- a/tests/CodeIndex.Tests/McpAuditLogTests.cs +++ b/tests/CodeIndex.Tests/McpAuditLogTests.cs @@ -67,6 +67,46 @@ public void ToolsCall_Ping_EmitsAuditRecordWithCallerFromInitialize() Assert.Equal(0, record.GetProperty("arg_keys").GetArrayLength()); } + [Fact] + public void ToolsCall_Ping_TruncatesClientInfoCallerFields_Issue3120() + { + using var sink = new AuditLogSink(_auditPath, AuditLogSink.DefaultMaxBytes, includeValues: false); + using var server = CreateServer(sink); + var name = new string('n', McpBoundedText.MaxClientInfoChars + 25); + var version = new string('v', McpBoundedText.MaxClientInfoChars + 25); + var nameDisplay = McpBoundedText.ForDisplay(name, McpBoundedText.MaxClientInfoChars); + var versionDisplay = McpBoundedText.ForDisplay(version, McpBoundedText.MaxClientInfoChars); + var init = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 1, + ["method"] = "initialize", + ["params"] = new JsonObject + { + ["clientInfo"] = new JsonObject + { + ["name"] = name, + ["version"] = version, + }, + }, + }; + _ = server.HandleMessage(init); + + var ping = JsonNode.Parse("""{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"ping","arguments":{}}}""")!; + _ = server.HandleMessage(ping); + + var rawLog = File.ReadAllText(_auditPath); + Assert.DoesNotContain(name, rawLog, StringComparison.Ordinal); + Assert.DoesNotContain(version, rawLog, StringComparison.Ordinal); + var record = ReadOnlyRecord(); + Assert.Equal(nameDisplay.Text, record.GetProperty("caller").GetString()); + Assert.Equal(name.Length, record.GetProperty("caller_length").GetInt32()); + Assert.True(record.GetProperty("caller_truncated").GetBoolean()); + Assert.Equal(versionDisplay.Text, record.GetProperty("caller_version").GetString()); + Assert.Equal(version.Length, record.GetProperty("caller_version_length").GetInt32()); + Assert.True(record.GetProperty("caller_version_truncated").GetBoolean()); + } + [Fact] public void ToolsCall_MissingToolName_StillEmitsAuditRecord() { diff --git a/tests/CodeIndex.Tests/McpServerTests.cs b/tests/CodeIndex.Tests/McpServerTests.cs index 1994a18142..5409a88442 100644 --- a/tests/CodeIndex.Tests/McpServerTests.cs +++ b/tests/CodeIndex.Tests/McpServerTests.cs @@ -725,6 +725,45 @@ public void Initialize_CapturesClientCapabilitiesAndRootsForSessionStatus() Assert.Equal("info", session["log_level"]!.GetValue()); } + [Fact] + public void Initialize_ClientInfo_TruncatesSessionStatusAndCallerIdentity_Issue3120() + { + var name = new string('n', McpBoundedText.MaxClientInfoChars + 25); + var version = new string('v', McpBoundedText.MaxClientInfoChars + 25); + var nameDisplay = McpBoundedText.ForDisplay(name, McpBoundedText.MaxClientInfoChars); + var versionDisplay = McpBoundedText.ForDisplay(version, McpBoundedText.MaxClientInfoChars); + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 1, + ["method"] = "initialize", + ["params"] = new JsonObject + { + ["clientInfo"] = new JsonObject + { + ["name"] = name, + ["version"] = version, + }, + }, + }; + + _server.HandleMessage(request); + + Assert.Equal($"{nameDisplay.Text}/{versionDisplay.Text}", _server.CurrentCaller); + var status = JsonNode.Parse("""{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"status","arguments":{}}}""")!; + var response = _server.HandleMessage(status)!; + + Assert.DoesNotContain(name, response.ToJsonString(), StringComparison.Ordinal); + Assert.DoesNotContain(version, response.ToJsonString(), StringComparison.Ordinal); + var clientInfo = response["result"]!["structuredContent"]!["mcp_session"]!["client_info"]!; + Assert.Equal(nameDisplay.Text, clientInfo["name"]!.GetValue()); + Assert.Equal(name.Length, clientInfo["name_length"]!.GetValue()); + Assert.True(clientInfo["name_truncated"]!.GetValue()); + Assert.Equal(versionDisplay.Text, clientInfo["version"]!.GetValue()); + Assert.Equal(version.Length, clientInfo["version_length"]!.GetValue()); + Assert.True(clientInfo["version_truncated"]!.GetValue()); + } + [Fact] public void LoggingSetLevel_UpdatesSessionLogLevel() { @@ -11287,6 +11326,22 @@ public void BuildCallerSwapRejectionLog_IsActionable() Assert.Contains("second-client", log); } + [Fact] + public void BuildCallerSwapRejectionLog_TruncatesClientInfoIdentities_Issue3120() + { + var current = new string('c', McpBoundedText.MaxClientIdentityChars + 25); + var attempted = new string('a', McpBoundedText.MaxClientIdentityChars + 25); + var currentDisplay = McpBoundedText.ForDisplay(current, McpBoundedText.MaxClientIdentityChars); + var attemptedDisplay = McpBoundedText.ForDisplay(attempted, McpBoundedText.MaxClientIdentityChars); + + var log = McpServer.BuildCallerSwapRejectionLog(current, attempted); + + Assert.DoesNotContain(current, log, StringComparison.Ordinal); + Assert.DoesNotContain(attempted, log, StringComparison.Ordinal); + Assert.Contains(currentDisplay.Text, log); + Assert.Contains(attemptedDisplay.Text, log); + } + [Fact] public void BatchQuery_RejectsNestedBatchQuerySlots() { @@ -11485,6 +11540,24 @@ public void RateLimited_Error_AlsoCarriesCanonicalEnvelope() AssertEnvelope(data, "rate_limited", expectedRetrySafe: true); } + [Fact] + public void RateLimited_ErrorAndLog_TruncatesCallerIdentity_Issue3120() + { + var caller = new string('r', McpBoundedText.MaxClientIdentityChars + 25); + var display = McpBoundedText.ForDisplay(caller, McpBoundedText.MaxClientIdentityChars); + + var response = McpServer.CreateRateLimitedErrorResponse(null, "status", caller, retryAfterMs: 123); + var log = McpServer.BuildRateLimitedLog("status", caller, retryAfterMs: 123); + + Assert.DoesNotContain(caller, response.ToJsonString(), StringComparison.Ordinal); + Assert.DoesNotContain(caller, log, StringComparison.Ordinal); + var data = response["error"]!["data"]!; + Assert.Equal(display.Text, data["caller"]!.GetValue()); + Assert.Equal(caller.Length, data["caller_length"]!.GetValue()); + Assert.True(data["caller_truncated"]!.GetValue()); + Assert.Contains(display.Text, log); + } + [Fact] public void ToolResult_DatabaseMissing_CarriesEnvelopeOnStructuredContent() { From 2c63dc562e216de3e8027480a0216a817fa5fc64 Mon Sep 17 00:00:00 2001 From: Widthdom Date: Thu, 4 Jun 2026 23:10:14 +0900 Subject: [PATCH 06/11] Bound MCP prompt diagnostics (#3121) --- changelog.d/unreleased/3121.security.md | 17 ++++ src/CodeIndex/Mcp/McpBoundedText.cs | 2 + src/CodeIndex/Mcp/McpServer.cs | 101 ++++++++++++++++++++---- tests/CodeIndex.Tests/McpServerTests.cs | 64 +++++++++++++++ 4 files changed, 169 insertions(+), 15 deletions(-) create mode 100644 changelog.d/unreleased/3121.security.md diff --git a/changelog.d/unreleased/3121.security.md b/changelog.d/unreleased/3121.security.md new file mode 100644 index 0000000000..4178f885f8 --- /dev/null +++ b/changelog.d/unreleased/3121.security.md @@ -0,0 +1,17 @@ +--- +category: security +issues: + - 3121 +affected: + - src/CodeIndex/Mcp/McpBoundedText.cs + - src/CodeIndex/Mcp/McpServer.cs + - tests/CodeIndex.Tests/McpServerTests.cs +--- + +## English + +- **MCP prompt names and arguments are bounded before prompt generation (#3121)** — `prompts/get` now rejects oversized prompt names or argument strings before embedding them in prompt text or diagnostics, and errors return bounded display values with truncation metadata. + +## 日本語 + +- **MCP prompt の name と argument を生成前に制限するようになりました (#3121)** — `prompts/get` は巨大な prompt name や argument 文字列を prompt text / 診断へ埋め込む前に拒否し、エラーには bounded display 値と truncation metadata を返します。 diff --git a/src/CodeIndex/Mcp/McpBoundedText.cs b/src/CodeIndex/Mcp/McpBoundedText.cs index 161cdfc893..2736650a9a 100644 --- a/src/CodeIndex/Mcp/McpBoundedText.cs +++ b/src/CodeIndex/Mcp/McpBoundedText.cs @@ -23,6 +23,8 @@ internal static class McpBoundedText internal const int MaxProtocolVersionChars = 128; internal const int MaxClientInfoChars = 128; internal const int MaxClientIdentityChars = (MaxClientInfoChars * 2) + 1; + internal const int MaxPromptNameChars = 128; + internal const int MaxPromptArgumentChars = 512; internal static BoundedMcpText ForDisplay(string value, int maxChars = MaxDiagnosticDisplayChars) { diff --git a/src/CodeIndex/Mcp/McpServer.cs b/src/CodeIndex/Mcp/McpServer.cs index 115789de63..9e1524cf4e 100644 --- a/src/CodeIndex/Mcp/McpServer.cs +++ b/src/CodeIndex/Mcp/McpServer.cs @@ -1965,25 +1965,63 @@ private JsonNode HandlePromptsGet(JsonNode? id, JsonNode? getParams) category: McpErrorEnvelope.CategoryMissingParameter, suggestion: "prompts/get requires `params.name`; call prompts/list to enumerate available names.", retrySafe: false); + name = name.Trim(); + if (name.Length > McpBoundedText.MaxPromptNameChars) + return CreatePromptStringTooLongError(id, parameterName: "name", value: name, maxChars: McpBoundedText.MaxPromptNameChars, + messagePrefix: "Prompt name is too long", + suggestion: "Use one of the short prompt names returned by prompts/list."); var args = getParams?["arguments"] as JsonObject; - string? ReadArg(string key) - => args != null && args.TryGetPropertyValue(key, out var node) && node is JsonValue value && value.TryGetValue(out var s) - ? s - : null; + string? ReadArg(string key, out JsonNode? error) + { + error = null; + if (args == null + || !args.TryGetPropertyValue(key, out var node) + || node is not JsonValue value + || !value.TryGetValue(out var s)) + { + return null; + } + if (s.Length > McpBoundedText.MaxPromptArgumentChars) + { + error = CreatePromptStringTooLongError(id, parameterName: key, value: s, maxChars: McpBoundedText.MaxPromptArgumentChars, + messagePrefix: $"Prompt argument '{key}' is too long", + suggestion: "Shorten prompt arguments before calling prompts/get; long source or path context should be fetched with tools instead."); + return null; + } + return McpBoundedText.ForDisplay(s, McpBoundedText.MaxPromptArgumentChars).Text; + } - var text = name switch + string text; + switch (name) { - "summarize_file" => $"Use the `outline` tool for `{ReadArg("path") ?? ""}`, then use `excerpt` only for the ranges needed to summarize public API, key symbols, and responsibilities.", - "find_unused" => $"Use `unused_symbols` with the requested scope `{ReadArg("scope") ?? ""}`. Cross-check surprising results with `references` or `callers` before recommending deletions.", - "impact_of_changing" => $"Use `impact_analysis` for `{ReadArg("symbol") ?? ""}`. Summarize direct callers, transitive callers, and files that likely need tests.", - _ => null, - }; - if (text == null) - return CreateErrorResponse(hasId: true, id: id, code: -32602, message: $"Unknown prompt: {name}", - category: McpErrorEnvelope.CategoryInvalidArgument, - suggestion: "Call prompts/list and request one of the advertised prompt names.", - retrySafe: false); + case "summarize_file": + { + var path = ReadArg("path", out var argumentError); + if (argumentError is not null) + return argumentError; + text = $"Use the `outline` tool for `{path ?? ""}`, then use `excerpt` only for the ranges needed to summarize public API, key symbols, and responsibilities."; + break; + } + case "find_unused": + { + var scope = ReadArg("scope", out var argumentError); + if (argumentError is not null) + return argumentError; + text = $"Use `unused_symbols` with the requested scope `{scope ?? ""}`. Cross-check surprising results with `references` or `callers` before recommending deletions."; + break; + } + case "impact_of_changing": + { + var symbol = ReadArg("symbol", out var argumentError); + if (argumentError is not null) + return argumentError; + text = $"Use `impact_analysis` for `{symbol ?? ""}`. Summarize direct callers, transitive callers, and files that likely need tests."; + break; + } + default: + return CreateUnknownPromptError(id, name); + } var messages = new JsonArray { @@ -2004,6 +2042,39 @@ private JsonNode HandlePromptsGet(JsonNode? id, JsonNode? getParams) }); } + private static JsonNode CreatePromptStringTooLongError(JsonNode? id, string parameterName, string value, int maxChars, string messagePrefix, string suggestion) + { + var display = McpBoundedText.ForDisplay(value, maxChars); + var data = new JsonObject + { + ["parameter"] = parameterName, + ["max_length"] = maxChars, + ["actual_length"] = value.Length, + ["value"] = display.Text, + }; + display.AddMetadata(data, "value"); + return CreateErrorResponse(hasId: true, id: id, code: -32602, message: $"{messagePrefix}: '{display.Text}'", + category: McpErrorEnvelope.CategoryInvalidArgument, + suggestion: suggestion, + retrySafe: false, + extraData: data); + } + + private static JsonNode CreateUnknownPromptError(JsonNode? id, string name) + { + var display = McpBoundedText.ForDisplay(name, McpBoundedText.MaxPromptNameChars); + var data = new JsonObject + { + ["prompt"] = display.Text, + }; + display.AddMetadata(data, "prompt"); + return CreateErrorResponse(hasId: true, id: id, code: -32602, message: $"Unknown prompt: {display.Text}", + category: McpErrorEnvelope.CategoryInvalidArgument, + suggestion: "Call prompts/list and request one of the advertised prompt names.", + retrySafe: false, + extraData: data); + } + private JsonNode HandleLoggingSetLevel(JsonNode? id, JsonNode? setLevelParams) { var level = TryReadStringValue(setLevelParams?["level"]); diff --git a/tests/CodeIndex.Tests/McpServerTests.cs b/tests/CodeIndex.Tests/McpServerTests.cs index 5409a88442..babd8cbfdf 100644 --- a/tests/CodeIndex.Tests/McpServerTests.cs +++ b/tests/CodeIndex.Tests/McpServerTests.cs @@ -865,6 +865,70 @@ public void PromptsListAndGet_ReturnPromptMessages() Assert.Contains("Run", message["content"]!["text"]!.GetValue()); } + [Fact] + public void PromptsGet_PromptNameTooLong_TruncatesDiagnostics_Issue3121() + { + var name = new string('p', McpBoundedText.MaxPromptNameChars + 25); + var display = McpBoundedText.ForDisplay(name, McpBoundedText.MaxPromptNameChars); + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 1, + ["method"] = "prompts/get", + ["params"] = new JsonObject + { + ["name"] = name, + }, + }; + + var response = _server.HandleMessage(request)!; + + Assert.Equal(-32602, response["error"]!["code"]!.GetValue()); + Assert.DoesNotContain(name, response.ToJsonString(), StringComparison.Ordinal); + Assert.Equal($"Prompt name is too long: '{display.Text}'", response["error"]!["message"]!.GetValue()); + var data = response["error"]!["data"]!; + Assert.Equal("name", data["parameter"]!.GetValue()); + Assert.Equal(McpBoundedText.MaxPromptNameChars, data["max_length"]!.GetValue()); + Assert.Equal(name.Length, data["actual_length"]!.GetValue()); + Assert.Equal(display.Text, data["value"]!.GetValue()); + Assert.Equal(name.Length, data["value_length"]!.GetValue()); + Assert.True(data["value_truncated"]!.GetValue()); + } + + [Fact] + public void PromptsGet_ArgumentValueTooLong_RejectsBeforePromptInterpolation_Issue3121() + { + var path = new string('x', McpBoundedText.MaxPromptArgumentChars + 25); + var display = McpBoundedText.ForDisplay(path, McpBoundedText.MaxPromptArgumentChars); + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 1, + ["method"] = "prompts/get", + ["params"] = new JsonObject + { + ["name"] = "summarize_file", + ["arguments"] = new JsonObject + { + ["path"] = path, + }, + }, + }; + + var response = _server.HandleMessage(request)!; + + Assert.Equal(-32602, response["error"]!["code"]!.GetValue()); + Assert.DoesNotContain(path, response.ToJsonString(), StringComparison.Ordinal); + Assert.Equal($"Prompt argument 'path' is too long: '{display.Text}'", response["error"]!["message"]!.GetValue()); + var data = response["error"]!["data"]!; + Assert.Equal("path", data["parameter"]!.GetValue()); + Assert.Equal(McpBoundedText.MaxPromptArgumentChars, data["max_length"]!.GetValue()); + Assert.Equal(path.Length, data["actual_length"]!.GetValue()); + Assert.Equal(display.Text, data["value"]!.GetValue()); + Assert.Equal(path.Length, data["value_length"]!.GetValue()); + Assert.True(data["value_truncated"]!.GetValue()); + } + [Fact] public void Initialize_RequestedCurrentProtocolVersion_EchoesBack() { From 0adff1808272ad1e79bda5f7e8063b0804abb13b Mon Sep 17 00:00:00 2001 From: Widthdom Date: Thu, 4 Jun 2026 23:20:50 +0900 Subject: [PATCH 07/11] Bound MCP resource URI diagnostics (#3122) --- changelog.d/unreleased/3122.security.md | 17 ++++++++++ src/CodeIndex/Mcp/McpBoundedText.cs | 1 + src/CodeIndex/Mcp/McpServer.cs | 31 +++++++++++++++--- tests/CodeIndex.Tests/McpServerTests.cs | 43 +++++++++++++++++++++++++ 4 files changed, 88 insertions(+), 4 deletions(-) create mode 100644 changelog.d/unreleased/3122.security.md diff --git a/changelog.d/unreleased/3122.security.md b/changelog.d/unreleased/3122.security.md new file mode 100644 index 0000000000..243500350d --- /dev/null +++ b/changelog.d/unreleased/3122.security.md @@ -0,0 +1,17 @@ +--- +category: security +issues: + - 3122 +affected: + - src/CodeIndex/Mcp/McpBoundedText.cs + - src/CodeIndex/Mcp/McpServer.cs + - tests/CodeIndex.Tests/McpServerTests.cs +--- + +## English + +- **MCP resources/read caps resource URIs before parsing (#3122)** — `resources/read` now rejects oversized URIs before URI parsing or unescaping and echoes bounded URI display values in invalid and not-found diagnostics. + +## 日本語 + +- **MCP resources/read が URI parse 前に長さ制限するようになりました (#3122)** — `resources/read` は巨大 URI を URI parse / unescape 前に拒否し、invalid / not-found 診断には bounded URI display 値を返します。 diff --git a/src/CodeIndex/Mcp/McpBoundedText.cs b/src/CodeIndex/Mcp/McpBoundedText.cs index 2736650a9a..d78a292537 100644 --- a/src/CodeIndex/Mcp/McpBoundedText.cs +++ b/src/CodeIndex/Mcp/McpBoundedText.cs @@ -25,6 +25,7 @@ internal static class McpBoundedText internal const int MaxClientIdentityChars = (MaxClientInfoChars * 2) + 1; internal const int MaxPromptNameChars = 128; internal const int MaxPromptArgumentChars = 512; + internal const int MaxResourceUriChars = 4096; internal static BoundedMcpText ForDisplay(string value, int maxChars = MaxDiagnosticDisplayChars) { diff --git a/src/CodeIndex/Mcp/McpServer.cs b/src/CodeIndex/Mcp/McpServer.cs index 9e1524cf4e..3cb5c5a6f6 100644 --- a/src/CodeIndex/Mcp/McpServer.cs +++ b/src/CodeIndex/Mcp/McpServer.cs @@ -1915,10 +1915,14 @@ private JsonNode HandleResourcesRead(JsonNode? id, JsonNode? readParams) category: McpErrorEnvelope.CategoryMissingParameter, suggestion: "resources/read requires `params.uri` from resources/list, such as `cdidx://file/src/app.cs`.", retrySafe: false); + if (uri.Length > McpBoundedText.MaxResourceUriChars) + return CreateResourceUriError(id, uri, messagePrefix: "Resource uri is too long", + suggestion: "Use a resource URI returned by resources/list and keep it within the documented MCP resource URI length limit.", + retrySafe: false, + includeLengthLimit: true); if (!TryParseResourceUri(uri, out var path)) - return CreateErrorResponse(hasId: true, id: id, code: -32602, message: $"Invalid resource uri: {uri}", - category: McpErrorEnvelope.CategoryInvalidArgument, + return CreateResourceUriError(id, uri, messagePrefix: "Invalid resource uri", suggestion: "Use a cdidx file resource URI returned by resources/list (`cdidx://file/`).", retrySafe: false); @@ -1927,8 +1931,7 @@ private JsonNode HandleResourcesRead(JsonNode? id, JsonNode? readParams) var files = reader.ListFiles(query: path, limit: 2); var file = files.FirstOrDefault(f => string.Equals(f.Path, path, StringComparison.Ordinal)); if (file == null) - return CreateErrorResponse(hasId: true, id: id, code: -32602, message: $"Resource not found: {uri}", - category: McpErrorEnvelope.CategoryInvalidArgument, + return CreateResourceUriError(id, uri, messagePrefix: "Resource not found", suggestion: "Call resources/list again and retry with one of the returned resource URIs.", retrySafe: true); @@ -1946,6 +1949,26 @@ private JsonNode HandleResourcesRead(JsonNode? id, JsonNode? readParams) }); } + private static JsonNode CreateResourceUriError(JsonNode? id, string uri, string messagePrefix, string suggestion, bool retrySafe, bool includeLengthLimit = false) + { + var display = McpBoundedText.ForDisplay(uri, McpBoundedText.MaxResourceUriChars); + var data = new JsonObject + { + ["uri"] = display.Text, + }; + display.AddMetadata(data, "uri"); + if (includeLengthLimit) + { + data["max_length"] = McpBoundedText.MaxResourceUriChars; + data["actual_length"] = uri.Length; + } + return CreateErrorResponse(hasId: true, id: id, code: -32602, message: $"{messagePrefix}: {display.Text}", + category: McpErrorEnvelope.CategoryInvalidArgument, + suggestion: suggestion, + retrySafe: retrySafe, + extraData: data); + } + private JsonNode HandlePromptsList(JsonNode? id) { var prompts = new JsonArray diff --git a/tests/CodeIndex.Tests/McpServerTests.cs b/tests/CodeIndex.Tests/McpServerTests.cs index babd8cbfdf..8dbf540163 100644 --- a/tests/CodeIndex.Tests/McpServerTests.cs +++ b/tests/CodeIndex.Tests/McpServerTests.cs @@ -844,6 +844,49 @@ public void ResourcesRead_NonStringUri_ReturnsInvalidParams() Assert.Equal("missing_parameter", response["error"]!["data"]!["category"]!.GetValue()); } + [Fact] + public void ResourcesRead_UriTooLong_RejectsBeforeParse_Issue3122() + { + var uri = "cdidx://file/" + new string('x', McpBoundedText.MaxResourceUriChars + 25); + var display = McpBoundedText.ForDisplay(uri, McpBoundedText.MaxResourceUriChars); + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 1, + ["method"] = "resources/read", + ["params"] = new JsonObject + { + ["uri"] = uri, + }, + }; + + var response = _server.HandleMessage(request)!; + + Assert.Equal(-32602, response["error"]!["code"]!.GetValue()); + Assert.DoesNotContain(uri, response.ToJsonString(), StringComparison.Ordinal); + Assert.Equal($"Resource uri is too long: {display.Text}", response["error"]!["message"]!.GetValue()); + var data = response["error"]!["data"]!; + Assert.Equal(display.Text, data["uri"]!.GetValue()); + Assert.Equal(uri.Length, data["uri_length"]!.GetValue()); + Assert.True(data["uri_truncated"]!.GetValue()); + Assert.Equal(McpBoundedText.MaxResourceUriChars, data["max_length"]!.GetValue()); + Assert.Equal(uri.Length, data["actual_length"]!.GetValue()); + } + + [Fact] + public void ResourcesRead_NotFound_ReturnsBoundedUriData_Issue3122() + { + const string uri = "cdidx://file/src/missing.cs"; + var request = JsonNode.Parse("""{"jsonrpc":"2.0","id":1,"method":"resources/read","params":{"uri":"cdidx://file/src/missing.cs"}}""")!; + + var response = _server.HandleMessage(request)!; + + Assert.Equal(-32602, response["error"]!["code"]!.GetValue()); + Assert.Equal($"Resource not found: {uri}", response["error"]!["message"]!.GetValue()); + Assert.Equal(uri, response["error"]!["data"]!["uri"]!.GetValue()); + Assert.Equal("invalid_argument", response["error"]!["data"]!["category"]!.GetValue()); + } + [Fact] public void PromptsListAndGet_ReturnPromptMessages() { From c6c57d84900e5960c146ae9412d536f606d2f950 Mon Sep 17 00:00:00 2001 From: Widthdom Date: Thu, 4 Jun 2026 23:51:00 +0900 Subject: [PATCH 08/11] Add MCP argument key truncation metadata (#3117) --- src/CodeIndex/Mcp/AuditLogSink.cs | 11 +++ src/CodeIndex/Mcp/McpServer.cs | 49 +++++++++----- tests/CodeIndex.Tests/McpAuditLogTests.cs | 42 +++++++++++- tests/CodeIndex.Tests/McpServerTests.cs | 82 ++++++++++++++++++++++- 4 files changed, 163 insertions(+), 21 deletions(-) diff --git a/src/CodeIndex/Mcp/AuditLogSink.cs b/src/CodeIndex/Mcp/AuditLogSink.cs index 869299a2eb..c8320146a9 100644 --- a/src/CodeIndex/Mcp/AuditLogSink.cs +++ b/src/CodeIndex/Mcp/AuditLogSink.cs @@ -239,6 +239,16 @@ internal static string SerializeEvent(AuditEvent evt, bool includeValues) jw.WriteNumber(kv.Key, kv.Value); jw.WriteEndObject(); + if (evt.ArgKeyLengths is { Count: > 0 } argKeyLengths) + { + jw.WritePropertyName("arg_key_lengths"); + jw.WriteStartObject(); + foreach (var kv in argKeyLengths) + jw.WriteNumber(kv.Key, kv.Value); + jw.WriteEndObject(); + jw.WriteBoolean("arg_keys_truncated", true); + } + if (includeValues && evt.ArgValues is { } values) { jw.WritePropertyName("arg_values"); @@ -287,6 +297,7 @@ internal sealed record AuditEvent( double ElapsedMs, int ErrorCode, string? ErrorType, + IReadOnlyList>? ArgKeyLengths = null, int? CallerNameLength = null, bool CallerNameTruncated = false, int? CallerVersionLength = null, diff --git a/src/CodeIndex/Mcp/McpServer.cs b/src/CodeIndex/Mcp/McpServer.cs index 3cb5c5a6f6..18cd59293b 100644 --- a/src/CodeIndex/Mcp/McpServer.cs +++ b/src/CodeIndex/Mcp/McpServer.cs @@ -2556,7 +2556,7 @@ private void EmitToolInvocationTelemetry(string toolName, JsonNode? args, JsonNo var context = CurrentCorrelationContext.Value; var (errorCode, observedErrorType) = ExtractErrorCode(response); var resultCount = ExtractResultCount(response); - var (argKeys, argLengths, _) = SanitizeArgs(args, includeValues: false); + var (argKeys, argLengths, argKeyLengths, _) = SanitizeArgs(args, includeValues: false); var toolDisplay = BoundToolNameForDisplay(toolName); var argsObject = new JsonObject(); foreach (var pair in argLengths) @@ -2578,6 +2578,7 @@ private void EmitToolInvocationTelemetry(string toolName, JsonNode? args, JsonNo ["arg_lengths"] = argsObject, }; toolDisplay.AddMetadata(evt, "tool"); + AddArgKeyMetadata(evt, argKeyLengths); DeferFrameLog(() => WriteMcpLogLine(evt.ToJsonString(_jsonOptions))); } @@ -2648,7 +2649,7 @@ private void TryEmitAudit(string toolName, JsonNode? id, JsonNode? args, JsonNod { var (errorCode, observedErrorType) = ExtractErrorCode(response); var resultCount = ExtractResultCount(response); - var (argKeys, argLengths, argValuesEcho) = SanitizeArgs(args, _auditLog.IncludeValues); + var (argKeys, argLengths, argKeyLengths, argValuesEcho) = SanitizeArgs(args, _auditLog.IncludeValues); var toolDisplay = BoundToolNameForDisplay(toolName); var evt = new AuditLogSink.AuditEvent( Timestamp: startedAt, @@ -2663,6 +2664,7 @@ private void TryEmitAudit(string toolName, JsonNode? id, JsonNode? args, JsonNod ElapsedMs: elapsedMs, ErrorCode: errorCode, ErrorType: errorType ?? observedErrorType, + ArgKeyLengths: argKeyLengths, CallerNameLength: _clientNameDisplay?.Truncated == true ? _clientNameDisplay.Value.OriginalLength : null, CallerNameTruncated: _clientNameDisplay?.Truncated == true, CallerVersionLength: _clientVersionDisplay?.Truncated == true ? _clientVersionDisplay.Value.OriginalLength : null, @@ -2734,7 +2736,7 @@ internal static (int Code, string? Type) ExtractErrorCode(JsonNode response) } /// - /// Build the `(arg_keys, arg_lengths, arg_values?)` audit triple. Values are echoed + /// Build the `(arg_keys, arg_lengths, arg_key_lengths, arg_values?)` audit triple. Values are echoed /// only when the operator has opted in via `--audit-log-include-values`; otherwise we /// keep keys + per-key length so AI argument shapes can be reconstructed without /// persisting query bodies that may contain sensitive substrings (#1562). @@ -2742,34 +2744,47 @@ internal static (int Code, string? Type) ExtractErrorCode(JsonNode response) /// `--audit-log-include-values` がオンの場合のみ転写し、それ以外はキーと長さだけ残す /// （secret 風の検索クエリを取り込まないため）。 /// - internal static (IReadOnlyList Keys, IReadOnlyList> Lengths, JsonNode? ValuesEcho) + internal static (IReadOnlyList Keys, IReadOnlyList> Lengths, IReadOnlyList> KeyLengths, JsonNode? ValuesEcho) SanitizeArgs(JsonNode? args, bool includeValues) { if (args is not JsonObject argsObj) - return (Array.Empty(), Array.Empty>(), null); + return (Array.Empty(), Array.Empty>(), Array.Empty>(), null); var keys = new List(argsObj.Count); var lengths = new List>(argsObj.Count); + var keyLengths = new List>(); + JsonObject? echoObject = includeValues ? new JsonObject() : null; foreach (var (key, value) in argsObj) { var keyDisplay = McpBoundedText.ForDisplay(key); keys.Add(keyDisplay.Text); lengths.Add(new KeyValuePair(keyDisplay.Text, AuditLogSink.MeasureArgLength(value))); - } - - JsonNode? echo = null; - if (includeValues) - { - try + if (keyDisplay.Truncated) + keyLengths.Add(new KeyValuePair(keyDisplay.Text, keyDisplay.OriginalLength)); + if (echoObject is not null) { - echo = argsObj.DeepClone(); - } - catch - { - echo = null; + try + { + echoObject[keyDisplay.Text] = value?.DeepClone(); + } + catch + { + echoObject = null; + } } } - return (keys, lengths, echo); + return (keys, lengths, keyLengths, includeValues ? echoObject : null); + } + + private static void AddArgKeyMetadata(JsonObject target, IReadOnlyList> argKeyLengths) + { + if (argKeyLengths.Count == 0) + return; + var lengths = new JsonObject(); + foreach (var pair in argKeyLengths) + lengths[pair.Key] = pair.Value; + target["arg_key_lengths"] = lengths; + target["arg_keys_truncated"] = true; } private static string? SerializeRequestId(JsonNode? id) diff --git a/tests/CodeIndex.Tests/McpAuditLogTests.cs b/tests/CodeIndex.Tests/McpAuditLogTests.cs index e31c3c55e7..2370bb0038 100644 --- a/tests/CodeIndex.Tests/McpAuditLogTests.cs +++ b/tests/CodeIndex.Tests/McpAuditLogTests.cs @@ -226,6 +226,40 @@ public void ToolsCall_UnknownTool_TruncatesAuditToolName_Issue3118() Assert.Equal(-32602, record.GetProperty("error_code").GetInt32()); } + [Fact] + public void ToolsCall_IncludeValues_TruncatesArgumentKeysInAuditValues_Issue3117() + { + using var sink = new AuditLogSink(_auditPath, AuditLogSink.DefaultMaxBytes, includeValues: true); + using var server = CreateServer(sink); + var argumentName = new string('k', McpBoundedText.MaxDiagnosticDisplayChars + 25); + var display = McpBoundedText.ForDisplay(argumentName); + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 1, + ["method"] = "tools/call", + ["params"] = new JsonObject + { + ["name"] = "index", + ["arguments"] = new JsonObject + { + ["path"] = ".", + [argumentName] = "value", + }, + }, + }; + + _ = server.HandleMessage(request); + + var rawLog = File.ReadAllText(_auditPath); + Assert.DoesNotContain(argumentName, rawLog, StringComparison.Ordinal); + var record = ReadOnlyRecord(); + Assert.Equal(display.Text, record.GetProperty("arg_keys")[1].GetString()); + Assert.Equal(argumentName.Length, record.GetProperty("arg_key_lengths").GetProperty(display.Text).GetInt32()); + Assert.True(record.GetProperty("arg_keys_truncated").GetBoolean()); + Assert.True(record.GetProperty("arg_values").TryGetProperty(display.Text, out _)); + } + [Fact] public void ToolsCall_IncludeValues_EchoesArgValuesIntoRecord() { @@ -311,9 +345,10 @@ public void ExtractResultCount_ReturnsNullForToolError() [Fact] public void SanitizeArgs_NullArgs_ReturnsEmptyTriple() { - var (keys, lengths, echo) = McpServer.SanitizeArgs(null, includeValues: true); + var (keys, lengths, keyLengths, echo) = McpServer.SanitizeArgs(null, includeValues: true); Assert.Empty(keys); Assert.Empty(lengths); + Assert.Empty(keyLengths); Assert.Null(echo); } @@ -321,10 +356,11 @@ public void SanitizeArgs_NullArgs_ReturnsEmptyTriple() public void SanitizeArgs_IncludeValuesFalse_ReturnsKeysAndLengthsOnly() { var args = JsonNode.Parse("""{"query":"hello","items":[1,2,3]}"""); - var (keys, lengths, echo) = McpServer.SanitizeArgs(args, includeValues: false); + var (keys, lengths, keyLengths, echo) = McpServer.SanitizeArgs(args, includeValues: false); Assert.Equal(new[] { "query", "items" }, keys); Assert.Equal(5, lengths.Single(kv => kv.Key == "query").Value); Assert.Equal(3, lengths.Single(kv => kv.Key == "items").Value); + Assert.Empty(keyLengths); Assert.Null(echo); } @@ -332,7 +368,7 @@ public void SanitizeArgs_IncludeValuesFalse_ReturnsKeysAndLengthsOnly() public void SanitizeArgs_IncludeValuesTrue_DeepClonesArgs() { var args = JsonNode.Parse("""{"query":"hello"}""")!.AsObject(); - var (_, _, echo) = McpServer.SanitizeArgs(args, includeValues: true); + var (_, _, _, echo) = McpServer.SanitizeArgs(args, includeValues: true); Assert.NotNull(echo); // Mutating the source must not change the cloned echo. // 元データを書き換えてもクローンに影響しないこと。 diff --git a/tests/CodeIndex.Tests/McpServerTests.cs b/tests/CodeIndex.Tests/McpServerTests.cs index ece3a5e6d0..23f681a8af 100644 --- a/tests/CodeIndex.Tests/McpServerTests.cs +++ b/tests/CodeIndex.Tests/McpServerTests.cs @@ -585,6 +585,62 @@ await Task.Run(() => Assert.Equal(-32602, root.GetProperty("error_code").GetInt32()); } + [Fact] + public async Task ProcessLineAsync_UnknownArgumentName_TruncatesTelemetryKeyMetadata_Issue3117() + { + using var writer = new StringWriter(); + using var error = new StringWriter(); + var argumentName = new string('k', McpBoundedText.MaxDiagnosticDisplayChars + 25); + var display = McpBoundedText.ForDisplay(argumentName); + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 123, + ["method"] = "tools/call", + ["params"] = new JsonObject + { + ["name"] = "index", + ["arguments"] = new JsonObject + { + ["path"] = ".", + [argumentName] = true, + }, + }, + }; + + await Task.Run(() => + { + lock (TestConsoleLock.Gate) + { + var previousError = Console.Error; + try + { + Console.SetError(error); +#pragma warning disable xUnit1031 + _server.ProcessLineAsync(request.ToJsonString(), writer).GetAwaiter().GetResult(); +#pragma warning restore xUnit1031 + } + finally + { + Console.SetError(previousError); + } + } + }); + + Assert.DoesNotContain(argumentName, writer.ToString(), StringComparison.Ordinal); + Assert.DoesNotContain(argumentName, error.ToString(), StringComparison.Ordinal); + var line = error.ToString() + .Split('\n', StringSplitOptions.RemoveEmptyEntries) + .Single(l => l.Contains("\"event\":\"mcp.tool.invocation\"", StringComparison.Ordinal)); + var jsonStart = line.IndexOf('{'); + using var document = JsonDocument.Parse(line[jsonStart..]); + var root = document.RootElement; + Assert.Equal("mcp.tool.invocation", root.GetProperty("event").GetString()); + Assert.Contains(root.GetProperty("arg_keys").EnumerateArray(), key => key.GetString() == display.Text); + Assert.Equal(argumentName.Length, root.GetProperty("arg_key_lengths").GetProperty(display.Text).GetInt32()); + Assert.True(root.GetProperty("arg_keys_truncated").GetBoolean()); + } + [Fact] public async Task ProcessLineAsync_FallbackErrorIncludesCorrelationData() { @@ -7735,15 +7791,39 @@ public void SanitizeArgs_TruncatesArgumentKeysForAuditAndTelemetry_Issue3117() [argumentName] = "value", }; - var (keys, lengths, valuesEcho) = McpServer.SanitizeArgs(args, includeValues: false); + var (keys, lengths, keyLengths, valuesEcho) = McpServer.SanitizeArgs(args, includeValues: false); Assert.Equal([display.Text], keys); var length = Assert.Single(lengths); Assert.Equal(display.Text, length.Key); Assert.Equal("value".Length, length.Value); + var keyLength = Assert.Single(keyLengths); + Assert.Equal(display.Text, keyLength.Key); + Assert.Equal(argumentName.Length, keyLength.Value); Assert.Null(valuesEcho); } + [Fact] + public void SanitizeArgs_TruncatesArgumentKeysInValuesEcho_Issue3117() + { + var argumentName = new string('k', McpBoundedText.MaxDiagnosticDisplayChars + 25); + var display = McpBoundedText.ForDisplay(argumentName); + var args = new JsonObject + { + [argumentName] = "value", + }; + + var (_, _, keyLengths, valuesEcho) = McpServer.SanitizeArgs(args, includeValues: true); + + Assert.NotNull(valuesEcho); + var json = valuesEcho!.ToJsonString(); + Assert.DoesNotContain(argumentName, json, StringComparison.Ordinal); + Assert.Equal("value", valuesEcho[display.Text]!.GetValue()); + var keyLength = Assert.Single(keyLengths); + Assert.Equal(display.Text, keyLength.Key); + Assert.Equal(argumentName.Length, keyLength.Value); + } + [Fact] public void ToolsCall_Index_WhenDbLockHeld_ReturnsBusyError() { From 08db6ee85dbb363d7eacef65d8817a623ac3cfc1 Mon Sep 17 00:00:00 2001 From: Widthdom Date: Thu, 4 Jun 2026 23:51:59 +0900 Subject: [PATCH 09/11] Add MCP audit tool truncation metadata (#3118) --- src/CodeIndex/Mcp/AuditLogSink.cs | 6 ++++++ src/CodeIndex/Mcp/McpServer.cs | 2 ++ tests/CodeIndex.Tests/McpAuditLogTests.cs | 2 ++ 3 files changed, 10 insertions(+) diff --git a/src/CodeIndex/Mcp/AuditLogSink.cs b/src/CodeIndex/Mcp/AuditLogSink.cs index c8320146a9..79135f5f40 100644 --- a/src/CodeIndex/Mcp/AuditLogSink.cs +++ b/src/CodeIndex/Mcp/AuditLogSink.cs @@ -212,6 +212,10 @@ internal static string SerializeEvent(AuditEvent evt, bool includeValues) jw.WriteStartObject(); jw.WriteString("timestamp", evt.Timestamp.ToString("O", CultureInfo.InvariantCulture)); jw.WriteString("tool", evt.Tool); + if (evt.ToolLength is { } toolLength) + jw.WriteNumber("tool_length", toolLength); + if (evt.ToolTruncated) + jw.WriteBoolean("tool_truncated", true); if (evt.CallerName is { } caller) jw.WriteString("caller", caller); if (evt.CallerNameLength is { } callerLength) @@ -297,6 +301,8 @@ internal sealed record AuditEvent( double ElapsedMs, int ErrorCode, string? ErrorType, + int? ToolLength = null, + bool ToolTruncated = false, IReadOnlyList>? ArgKeyLengths = null, int? CallerNameLength = null, bool CallerNameTruncated = false, diff --git a/src/CodeIndex/Mcp/McpServer.cs b/src/CodeIndex/Mcp/McpServer.cs index 18cd59293b..e8da868c4e 100644 --- a/src/CodeIndex/Mcp/McpServer.cs +++ b/src/CodeIndex/Mcp/McpServer.cs @@ -2664,6 +2664,8 @@ private void TryEmitAudit(string toolName, JsonNode? id, JsonNode? args, JsonNod ElapsedMs: elapsedMs, ErrorCode: errorCode, ErrorType: errorType ?? observedErrorType, + ToolLength: toolDisplay.Truncated ? toolDisplay.OriginalLength : null, + ToolTruncated: toolDisplay.Truncated, ArgKeyLengths: argKeyLengths, CallerNameLength: _clientNameDisplay?.Truncated == true ? _clientNameDisplay.Value.OriginalLength : null, CallerNameTruncated: _clientNameDisplay?.Truncated == true, diff --git a/tests/CodeIndex.Tests/McpAuditLogTests.cs b/tests/CodeIndex.Tests/McpAuditLogTests.cs index 2370bb0038..b410989cda 100644 --- a/tests/CodeIndex.Tests/McpAuditLogTests.cs +++ b/tests/CodeIndex.Tests/McpAuditLogTests.cs @@ -223,6 +223,8 @@ public void ToolsCall_UnknownTool_TruncatesAuditToolName_Issue3118() Assert.DoesNotContain(toolName, rawLog, StringComparison.Ordinal); var record = ReadOnlyRecord(); Assert.Equal(display.Text, record.GetProperty("tool").GetString()); + Assert.Equal(toolName.Length, record.GetProperty("tool_length").GetInt32()); + Assert.True(record.GetProperty("tool_truncated").GetBoolean()); Assert.Equal(-32602, record.GetProperty("error_code").GetInt32()); } From 8408be9c62cf93a3039a7ed8fb229792b13014e5 Mon Sep 17 00:00:00 2001 From: Widthdom Date: Fri, 5 Jun 2026 00:22:34 +0900 Subject: [PATCH 10/11] Address MCP diagnostic review gaps (#3116 #3117 #3118) --- src/CodeIndex/Mcp/McpServer.cs | 68 ++++++++-- src/CodeIndex/Mcp/McpToolHandlers.cs | 31 ++++- tests/CodeIndex.Tests/McpServerTests.cs | 163 ++++++++++++++++++++++++ 3 files changed, 247 insertions(+), 15 deletions(-) diff --git a/src/CodeIndex/Mcp/McpServer.cs b/src/CodeIndex/Mcp/McpServer.cs index e8da868c4e..5a4eefc699 100644 --- a/src/CodeIndex/Mcp/McpServer.cs +++ b/src/CodeIndex/Mcp/McpServer.cs @@ -1,6 +1,8 @@ using System.Collections.Concurrent; using System.Diagnostics; +using System.Globalization; using System.Runtime.InteropServices; +using System.Security.Cryptography; using System.Text; using System.Text.Json; using System.Text.Json.Nodes; @@ -2406,11 +2408,17 @@ private async Task HandleToolsCallAsync(JsonNode? id, JsonNode? callPa var metricsStopwatch = System.Diagnostics.Stopwatch.StartNew(); string? metricsError = null; JsonNode response; + JsonObject CreateUnknownToolResponseForMetrics() + { + metricsError = "unknown_tool"; + return CreateUnknownToolErrorResponse(hasId: true, id: id, toolName); + } + try { if (toolNameTooLong) { - response = CreateUnknownToolErrorResponse(hasId: true, id: id, toolName); + response = CreateUnknownToolResponseForMetrics(); } else if (ValidateToolArguments(toolName, args) is JsonObject argumentError) { @@ -2487,7 +2495,7 @@ private async Task HandleToolsCallAsync(JsonNode? id, JsonNode? callPa "index" => await ExecuteIndexAsync(id, args, progressToken).ConfigureAwait(false), "backfill_fold" => ExecuteBackfillFold(id, args, progressToken), "suggest_improvement" => await ExecuteSuggestImprovementAsync(id, args).ConfigureAwait(false), - _ => CreateUnknownToolErrorResponse(hasId: true, id: id, toolName), + _ => CreateUnknownToolResponseForMetrics(), }; } } @@ -2528,13 +2536,14 @@ private async Task HandleToolsCallAsync(JsonNode? id, JsonNode? callPa if (MetricsSink.IsActive) { metricsStopwatch.Stop(); + var metricsTool = BoundToolNameForDisplay(toolName).Text; MetricsSink.Record(new MetricsEvent( Timestamp: metricsStartedAt, - Tool: toolName, + Tool: metricsTool, Source: "mcp", ElapsedMs: metricsStopwatch.Elapsed.TotalMilliseconds, ExitCode: metricsError == null ? 0 : 1, - Language: TryReadStringArg(args, "language") ?? TryReadStringArg(args, "lang"), + Language: TryReadMetricStringArg(args, "language") ?? TryReadMetricStringArg(args, "lang"), Error: metricsError)); } } @@ -2546,7 +2555,8 @@ private async Task HandleToolsCallAsync(JsonNode? id, JsonNode? callPa // audit はワイヤーレスポンスと例外型の両方を参照するため metrics finally の後で // 出力する。Stopwatch.Stop は冪等。TryEmitAudit 内部でベストエフォート化済み (#1562)。 metricsStopwatch.Stop(); - TryEmitAudit(toolName, id, args, response, metricsStartedAt, metricsStopwatch.Elapsed.TotalMilliseconds, errorType: metricsError); + var auditErrorType = metricsError == "unknown_tool" ? null : metricsError; + TryEmitAudit(toolName, id, args, response, metricsStartedAt, metricsStopwatch.Elapsed.TotalMilliseconds, errorType: auditErrorType); EmitToolInvocationTelemetry(toolName, args, response, metricsStartedAt, metricsStopwatch.Elapsed.TotalMilliseconds, metricsError); return response; } @@ -2755,19 +2765,21 @@ internal static (IReadOnlyList Keys, IReadOnlyList(argsObj.Count); var lengths = new List>(argsObj.Count); var keyLengths = new List>(); + var usedKeys = new HashSet(StringComparer.Ordinal); JsonObject? echoObject = includeValues ? new JsonObject() : null; foreach (var (key, value) in argsObj) { var keyDisplay = McpBoundedText.ForDisplay(key); - keys.Add(keyDisplay.Text); - lengths.Add(new KeyValuePair(keyDisplay.Text, AuditLogSink.MeasureArgLength(value))); + var displayKey = MakeUniqueArgumentDisplayKey(key, keyDisplay, usedKeys); + keys.Add(displayKey); + lengths.Add(new KeyValuePair(displayKey, AuditLogSink.MeasureArgLength(value))); if (keyDisplay.Truncated) - keyLengths.Add(new KeyValuePair(keyDisplay.Text, keyDisplay.OriginalLength)); + keyLengths.Add(new KeyValuePair(displayKey, keyDisplay.OriginalLength)); if (echoObject is not null) { try { - echoObject[keyDisplay.Text] = value?.DeepClone(); + echoObject[displayKey] = value?.DeepClone(); } catch { @@ -2778,6 +2790,38 @@ internal static (IReadOnlyList Keys, IReadOnlyList usedKeys) + { + if (usedKeys.Add(display.Text)) + return display.Text; + + var hashSuffix = "#" + ShortStableHash(rawKey); + var candidate = ComposeDisplayKeyWithSuffix(rawKey, hashSuffix); + var disambiguator = 2; + while (!usedKeys.Add(candidate)) + { + candidate = ComposeDisplayKeyWithSuffix( + rawKey, + $"{hashSuffix}-{disambiguator.ToString(CultureInfo.InvariantCulture)}"); + disambiguator++; + } + + return candidate; + } + + private static string ComposeDisplayKeyWithSuffix(string rawKey, string suffix) + { + const int maxDisplayTextChars = McpBoundedText.MaxDiagnosticDisplayChars + 3; + var maxPrefixChars = Math.Max(0, maxDisplayTextChars - suffix.Length - 3); + return McpBoundedText.ForDisplay(rawKey, maxPrefixChars).Text + suffix; + } + + private static string ShortStableHash(string value) + { + var bytes = SHA256.HashData(Encoding.UTF8.GetBytes(value)); + return Convert.ToHexString(bytes.AsSpan(0, 4)).ToLowerInvariant(); + } + private static void AddArgKeyMetadata(JsonObject target, IReadOnlyList> argKeyLengths) { if (argKeyLengths.Count == 0) @@ -2824,6 +2868,12 @@ private static void AddArgKeyMetadata(JsonObject target, IReadOnlyList $"[cdidx-mcp] Message too large ({characterCount} chars / {byteCount} bytes), rejecting. Split the request into smaller JSON-RPC messages or shorter arguments, then retry."; diff --git a/src/CodeIndex/Mcp/McpToolHandlers.cs b/src/CodeIndex/Mcp/McpToolHandlers.cs index 24dd30d60f..828d8c1a87 100644 --- a/src/CodeIndex/Mcp/McpToolHandlers.cs +++ b/src/CodeIndex/Mcp/McpToolHandlers.cs @@ -2534,8 +2534,23 @@ bool TryAppendResult(JsonObject entry, string? toolName, JsonNode? toolArgs, int return true; } + static void CopySlotErrorData(JsonObject entry, JsonObject? extraData) + { + if (extraData is null) + return; + + foreach (var (key, value) in extraData) + { + if (key is "message" or "jsonrpc_invalid_params") + continue; + if (entry.ContainsKey(key)) + continue; + entry[key] = value?.DeepClone(); + } + } + void AppendSlotError(int requestIndex, string? toolName, JsonNode? toolArgs, Stopwatch slotStopwatch, string errorMessage, - int? code = null, string? category = null, string? suggestion = null, bool? retrySafe = null) + int? code = null, string? category = null, string? suggestion = null, bool? retrySafe = null, JsonObject? extraData = null) { slotStopwatch.Stop(); var entry = new JsonObject @@ -2548,6 +2563,7 @@ void AppendSlotError(int requestIndex, string? toolName, JsonNode? toolArgs, Sto ["error"] = errorMessage, }; AddToolDisplayData(entry, toolName); + CopySlotErrorData(entry, extraData); if (code.HasValue) entry["code"] = code.Value; // #1581: batch_query slot errors also carry the canonical envelope so clients @@ -2654,7 +2670,8 @@ void AppendRateLimitedSlot(int requestIndex, string? toolName, JsonNode? toolArg AppendSlotError(requestIndex, toolName, toolArgs, slotStopwatch, argumentError["message"]!.GetValue(), category: McpErrorEnvelope.CategoryInvalidArgument, suggestion: "Use exactly the argument names advertised by tools/list for this tool.", - retrySafe: false); + retrySafe: false, + extraData: argumentError); continue; } @@ -2663,7 +2680,8 @@ void AppendRateLimitedSlot(int requestIndex, string? toolName, JsonNode? toolArg AppendSlotError(requestIndex, toolName, toolArgs, slotStopwatch, listArgumentError["message"]!.GetValue(), category: McpErrorEnvelope.CategoryInvalidArgument, suggestion: "Send only non-empty string entries within the documented MCP array bounds.", - retrySafe: false); + retrySafe: false, + extraData: listArgumentError); continue; } @@ -2981,15 +2999,16 @@ private static string BuildArgsSummary(JsonNode? toolArgs) var parts = new List(obj.Count); foreach (var kv in obj) { - var key = kv.Key; + var key = McpBoundedText.ForDisplay(kv.Key).Text; var value = kv.Value; string rendered = value switch { null => "null", - JsonValue v => v.ToJsonString(), + JsonValue v when v.TryGetValue(out var text) => JsonSerializer.Serialize(McpBoundedText.ForDisplay(text).Text), + JsonValue v => McpBoundedText.ForDisplay(v.ToJsonString()).Text, JsonArray arr => $"[{arr.Count}]", JsonObject inner => $"{{{inner.Count}}}", - _ => value.ToJsonString(), + _ => McpBoundedText.ForDisplay(value.ToJsonString()).Text, }; parts.Add($"{key}={rendered}"); } diff --git a/tests/CodeIndex.Tests/McpServerTests.cs b/tests/CodeIndex.Tests/McpServerTests.cs index 23f681a8af..95eb0e1fef 100644 --- a/tests/CodeIndex.Tests/McpServerTests.cs +++ b/tests/CodeIndex.Tests/McpServerTests.cs @@ -7383,6 +7383,97 @@ public void ToolsCall_BatchQuery_UnknownToolName_TruncatesDisplay_Issue3118() Assert.Equal($"Unknown tool: {display.Text}", slot["error"]!.GetValue()); } + [Fact] + public void ToolsCall_BatchQuery_UnknownArgumentName_CarriesTruncationMetadata_Issue3117() + { + var argumentName = new string('u', McpBoundedText.MaxDiagnosticDisplayChars + 25); + var display = McpBoundedText.ForDisplay(argumentName); + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 1, + ["method"] = "tools/call", + ["params"] = new JsonObject + { + ["name"] = "batch_query", + ["arguments"] = new JsonObject + { + ["queries"] = new JsonArray + { + new JsonObject + { + ["tool"] = "search", + ["arguments"] = new JsonObject + { + ["query"] = "App", + [argumentName] = 1, + }, + }, + }, + }, + }, + }; + + var response = _server.HandleMessage(request)!; + + Assert.DoesNotContain(argumentName, response.ToJsonString(), StringComparison.Ordinal); + var slot = Assert.Single(response["result"]!["structuredContent"]!["results"]!.AsArray())!; + Assert.False(slot["ok"]!.GetValue()); + Assert.Equal("search", slot["tool"]!.GetValue()); + Assert.Equal(display.Text, slot["unknown_argument"]!.GetValue()); + Assert.Equal(argumentName.Length, slot["unknown_argument_length"]!.GetValue()); + Assert.True(slot["unknown_argument_truncated"]!.GetValue()); + Assert.Contains(display.Text, slot["error"]!.GetValue()); + Assert.Contains(display.Text, slot["args_summary"]!.GetValue()); + } + + [Fact] + public void ToolsCall_BatchQuery_EnumLikeScalarTooLong_CarriesTruncationMetadata_Issue3116() + { + var oversized = new string('A', McpBoundedText.MaxScalarArgumentChars + 25); + var display = McpBoundedText.ForDisplay(oversized); + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 1, + ["method"] = "tools/call", + ["params"] = new JsonObject + { + ["name"] = "batch_query", + ["arguments"] = new JsonObject + { + ["queries"] = new JsonArray + { + new JsonObject + { + ["tool"] = "search", + ["arguments"] = new JsonObject + { + ["query"] = "App", + ["format"] = oversized, + }, + }, + }, + }, + }, + }; + + var response = _server.HandleMessage(request)!; + + Assert.DoesNotContain(oversized, response.ToJsonString(), StringComparison.Ordinal); + var slot = Assert.Single(response["result"]!["structuredContent"]!["results"]!.AsArray())!; + Assert.False(slot["ok"]!.GetValue()); + Assert.Equal("search", slot["tool"]!.GetValue()); + Assert.Equal("format", slot["parameter"]!.GetValue()); + Assert.Equal(display.Text, slot["value"]!.GetValue()); + Assert.Equal(McpBoundedText.MaxScalarArgumentChars, slot["max_length"]!.GetValue()); + Assert.Equal(oversized.Length, slot["actual_length"]!.GetValue()); + Assert.Equal(oversized.Length, slot["value_length"]!.GetValue()); + Assert.True(slot["value_truncated"]!.GetValue()); + Assert.Contains(display.Text, slot["error"]!.GetValue()); + Assert.Contains(display.Text, slot["args_summary"]!.GetValue()); + } + [Fact] public void ToolsCall_BatchQuery_SanitizesSlotExceptionMessage_Issue2849() { @@ -7781,6 +7872,52 @@ public void ToolsCall_UnknownToolName_TruncatesDisplay_Issue3118() Assert.True(data["tool_truncated"]!.GetValue()); } + [Fact] + public void ToolsCall_UnknownToolName_TruncatesMetricsLabels_Issue3118() + { + var toolName = new string('m', McpBoundedText.MaxToolNameChars + 25); + var language = new string('l', McpBoundedText.MaxDiagnosticDisplayChars + 25); + var toolDisplay = McpBoundedText.ForDisplay(toolName, McpBoundedText.MaxToolNameChars); + var languageDisplay = McpBoundedText.ForDisplay(language); + var metricsPath = Path.Combine(Path.GetTempPath(), $"cdidx_mcp_metrics_{Guid.NewGuid():N}.jsonl"); + try + { + using var session = MetricsSink.TryStartForTesting(metricsPath, maxBytes: 1024 * 1024); + Assert.NotNull(session); + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 1, + ["method"] = "tools/call", + ["params"] = new JsonObject + { + ["name"] = toolName, + ["arguments"] = new JsonObject + { + ["lang"] = language, + }, + }, + }; + + var response = _server.HandleMessage(request)!; + + Assert.Equal(-32602, response["error"]!["code"]!.GetValue()); + var line = Assert.Single(File.ReadAllLines(metricsPath)); + Assert.DoesNotContain(toolName, line, StringComparison.Ordinal); + Assert.DoesNotContain(language, line, StringComparison.Ordinal); + using var doc = JsonDocument.Parse(line); + var root = doc.RootElement; + Assert.Equal(toolDisplay.Text, root.GetProperty("tool").GetString()); + Assert.Equal(languageDisplay.Text, root.GetProperty("language").GetString()); + Assert.Equal(1, root.GetProperty("exit_code").GetInt32()); + Assert.Equal("unknown_tool", root.GetProperty("error").GetString()); + } + finally + { + DeleteFileRobust(metricsPath); + } + } + [Fact] public void SanitizeArgs_TruncatesArgumentKeysForAuditAndTelemetry_Issue3117() { @@ -7824,6 +7961,32 @@ public void SanitizeArgs_TruncatesArgumentKeysInValuesEcho_Issue3117() Assert.Equal(argumentName.Length, keyLength.Value); } + [Fact] + public void SanitizeArgs_DisambiguatesCollidingTruncatedKeys_Issue3117() + { + var sharedPrefix = new string('c', McpBoundedText.MaxDiagnosticDisplayChars + 25); + var firstArgumentName = sharedPrefix + "a"; + var secondArgumentName = sharedPrefix + "b"; + var args = new JsonObject + { + [firstArgumentName] = "one", + [secondArgumentName] = "two", + }; + + var (keys, lengths, keyLengths, valuesEcho) = McpServer.SanitizeArgs(args, includeValues: true); + + Assert.Equal(2, keys.Count); + Assert.Equal(2, keys.Distinct(StringComparer.Ordinal).Count()); + Assert.Equal(2, lengths.Select(pair => pair.Key).Distinct(StringComparer.Ordinal).Count()); + Assert.Equal(2, keyLengths.Select(pair => pair.Key).Distinct(StringComparer.Ordinal).Count()); + Assert.NotNull(valuesEcho); + var json = valuesEcho!.ToJsonString(); + Assert.DoesNotContain(firstArgumentName, json, StringComparison.Ordinal); + Assert.DoesNotContain(secondArgumentName, json, StringComparison.Ordinal); + Assert.Equal("one", valuesEcho[keys[0]]!.GetValue()); + Assert.Equal("two", valuesEcho[keys[1]]!.GetValue()); + } + [Fact] public void ToolsCall_Index_WhenDbLockHeld_ReturnsBusyError() { From bd52b16a8958418892a83bc597fb360d3780dfc7 Mon Sep 17 00:00:00 2001 From: Widthdom Date: Fri, 5 Jun 2026 00:37:48 +0900 Subject: [PATCH 11/11] Fix MCP prompt formatting (#3121) --- src/CodeIndex/Mcp/McpServer.cs | 42 +++++++++++++++++----------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/src/CodeIndex/Mcp/McpServer.cs b/src/CodeIndex/Mcp/McpServer.cs index 5a4eefc699..f61e202b88 100644 --- a/src/CodeIndex/Mcp/McpServer.cs +++ b/src/CodeIndex/Mcp/McpServer.cs @@ -2021,29 +2021,29 @@ private JsonNode HandlePromptsGet(JsonNode? id, JsonNode? getParams) switch (name) { case "summarize_file": - { - var path = ReadArg("path", out var argumentError); - if (argumentError is not null) - return argumentError; - text = $"Use the `outline` tool for `{path ?? ""}`, then use `excerpt` only for the ranges needed to summarize public API, key symbols, and responsibilities."; - break; - } + { + var path = ReadArg("path", out var argumentError); + if (argumentError is not null) + return argumentError; + text = $"Use the `outline` tool for `{path ?? ""}`, then use `excerpt` only for the ranges needed to summarize public API, key symbols, and responsibilities."; + break; + } case "find_unused": - { - var scope = ReadArg("scope", out var argumentError); - if (argumentError is not null) - return argumentError; - text = $"Use `unused_symbols` with the requested scope `{scope ?? ""}`. Cross-check surprising results with `references` or `callers` before recommending deletions."; - break; - } + { + var scope = ReadArg("scope", out var argumentError); + if (argumentError is not null) + return argumentError; + text = $"Use `unused_symbols` with the requested scope `{scope ?? ""}`. Cross-check surprising results with `references` or `callers` before recommending deletions."; + break; + } case "impact_of_changing": - { - var symbol = ReadArg("symbol", out var argumentError); - if (argumentError is not null) - return argumentError; - text = $"Use `impact_analysis` for `{symbol ?? ""}`. Summarize direct callers, transitive callers, and files that likely need tests."; - break; - } + { + var symbol = ReadArg("symbol", out var argumentError); + if (argumentError is not null) + return argumentError; + text = $"Use `impact_analysis` for `{symbol ?? ""}`. Summarize direct callers, transitive callers, and files that likely need tests."; + break; + } default: return CreateUnknownPromptError(id, name); }