diff --git a/USER_GUIDE.md b/USER_GUIDE.md index 73e97aaa33..db5649dbcf 100644 --- a/USER_GUIDE.md +++ b/USER_GUIDE.md @@ -2002,7 +2002,7 @@ When both are set, the allowlist wins. `tools/list` only advertises enabled tool #### MCP roots and sampling -`cdidx mcp` advertises roots and sampling support during `initialize`. When the client supports roots, `index` refreshes `roots/list` and rejects paths outside the granted client roots. `suggest_improvement` uses `sampling/createMessage` to extract an optional one-line title and tag list before storing the raw suggestion. Set `CDIDX_MCP_SAMPLING=0` (or `false` / `off`) to disable server-to-client sampling requests. +`cdidx mcp` advertises roots and sampling support during `initialize`. When the client supports roots, `index` refreshes `roots/list` and rejects paths outside the granted client roots. `suggest_improvement` uses `sampling/createMessage` to extract an optional one-line title and tag list before storing the raw suggestion. Sampling prompts are byte-bounded, long fields are clamped to one-line summaries, and `toolInvocationContext` is summarized without sending its raw content to the sampling client. Set `CDIDX_MCP_SAMPLING=0` (or `false` / `off`) to disable server-to-client sampling requests. ### Why cdidx over grep/ripgrep for AI workflows? @@ -4087,7 +4087,7 @@ stdio トランスポートはバイト単位で挙動が変わらないため #### MCP roots と sampling -`cdidx mcp` は `initialize` で roots と sampling support を広告します。クライアントが roots をサポートする場合、`index` は `roots/list` を更新し、許可された client root の外にある path を拒否します。`suggest_improvement` は raw suggestion を保存する前に `sampling/createMessage` で任意の 1 行タイトルとタグ一覧を抽出します。server-to-client sampling request を無効化するには `CDIDX_MCP_SAMPLING=0`（または `false` / `off`）を設定してください。 +`cdidx mcp` は `initialize` で roots と sampling support を広告します。クライアントが roots をサポートする場合、`index` は `roots/list` を更新し、許可された client root の外にある path を拒否します。`suggest_improvement` は raw suggestion を保存する前に `sampling/createMessage` で任意の 1 行タイトルとタグ一覧を抽出します。sampling prompt は byte 上限内に収められ、長い field は 1 行 summary に切り詰められ、`toolInvocationContext` は raw 内容を sampling client に送らず summary 化されます。server-to-client sampling request を無効化するには `CDIDX_MCP_SAMPLING=0`（または `false` / `off`）を設定してください。 ### AIワークフローで grep/ripgrep より cdidx が優れる理由 diff --git a/changelog.d/unreleased/2897.security.md b/changelog.d/unreleased/2897.security.md new file mode 100644 index 0000000000..52f09f3368 --- /dev/null +++ b/changelog.d/unreleased/2897.security.md @@ -0,0 +1,16 @@ +--- +category: security +issues: + - 2897 +affected: + - src/CodeIndex/Mcp/McpToolHandlers.cs + - tests/CodeIndex.Tests/McpServerTests.cs +--- + +## English + +- **MCP suggestion sampling now bounds sampled JSON before parsing (#2897)** — `suggest_improvement` ignores oversized or overly deep sampling responses instead of parsing unbounded client-provided metadata JSON. + +## 日本語 + +- **MCP suggestion sampling が sampled JSON を parse 前に制限するようになりました (#2897)** — `suggest_improvement` は sampling client から返った metadata JSON が大きすぎる、または深すぎる場合に、無制限に parse せず metadata enrichment を失敗扱いにします。 diff --git a/changelog.d/unreleased/2898.security.md b/changelog.d/unreleased/2898.security.md new file mode 100644 index 0000000000..e71e2137d2 --- /dev/null +++ b/changelog.d/unreleased/2898.security.md @@ -0,0 +1,17 @@ +--- +category: security +issues: + - 2898 +affected: + - src/CodeIndex/Mcp/McpToolHandlers.cs + - tests/CodeIndex.Tests/McpServerTests.cs + - USER_GUIDE.md +--- + +## English + +- **MCP suggestion sampling now bounds outbound prompts (#2898)** — `suggest_improvement` clamps sampling prompt fields, enforces a complete prompt byte budget, and summarizes `toolInvocationContext` without forwarding its raw content to the sampling client. + +## 日本語 + +- **MCP suggestion sampling が送信 prompt を制限するようになりました (#2898)** — `suggest_improvement` は sampling prompt の field を切り詰め、prompt 全体の byte budget を強制し、`toolInvocationContext` の raw 内容を sampling client に転送せず summary 化します。 diff --git a/src/CodeIndex/Mcp/McpToolHandlers.cs b/src/CodeIndex/Mcp/McpToolHandlers.cs index 514a49540a..dbf4f4cabb 100644 --- a/src/CodeIndex/Mcp/McpToolHandlers.cs +++ b/src/CodeIndex/Mcp/McpToolHandlers.cs @@ -3966,6 +3966,14 @@ private static JsonObject BuildBackfillProgressJson(int rowsDone, int rowsTotal) /// private const int MaxContextLength = 1000; + private const int MaxSamplingPromptBytes = 4096; + private const int MaxSamplingShortFieldChars = 80; + private const int MaxSamplingDescriptionChars = 800; + private const int MaxSamplingContextChars = 400; + private const int MaxSamplingToolInvocationSummaryChars = 160; + private const int MaxSamplingResponseTextChars = 8192; + private const int MaxSamplingResponseJsonDepth = 16; + /// /// Handle the suggest_improvement tool call. /// Records a structured suggestion to .cdidx/suggestions-*.json. @@ -4164,18 +4172,7 @@ private sealed record SuggestionSamplingResult(string? Title, string[]? Tags); if (!IsSamplingEnabled() || !HasClientCapability("sampling")) return null; - var prompt = new StringBuilder(); - prompt.AppendLine("Extract structured metadata for a cdidx improvement suggestion."); - prompt.AppendLine("Return only compact JSON with keys: title (one line, <=80 chars) and tags (array of 1-6 lowercase identifiers)."); - prompt.AppendLine("Do not include source code."); - prompt.AppendLine($"category: {category}"); - if (!string.IsNullOrWhiteSpace(language)) - prompt.AppendLine($"language: {language}"); - prompt.AppendLine($"description: {description}"); - if (!string.IsNullOrWhiteSpace(context)) - prompt.AppendLine($"context: {context}"); - if (!string.IsNullOrWhiteSpace(toolInvocationContext)) - prompt.AppendLine($"tool_invocation_context: {toolInvocationContext}"); + var prompt = BuildSuggestionSamplingPrompt(category, language, description, context, toolInvocationContext); var result = await SendClientRequestAsync("sampling/createMessage", new JsonObject { @@ -4187,7 +4184,7 @@ private sealed record SuggestionSamplingResult(string? Title, string[]? Tags); ["content"] = new JsonObject { ["type"] = "text", - ["text"] = prompt.ToString(), + ["text"] = prompt, } } }, @@ -4197,9 +4194,11 @@ private sealed record SuggestionSamplingResult(string? Title, string[]? Tags); var text = ExtractSamplingText(result); if (string.IsNullOrWhiteSpace(text)) return null; + if (text.Length > MaxSamplingResponseTextChars) + return null; try { - var parsed = JsonNode.Parse(text); + var parsed = JsonNode.Parse(text, documentOptions: new JsonDocumentOptions { MaxDepth = MaxSamplingResponseJsonDepth }); var title = SanitizeSampledTitle(TryReadStringValue(parsed?["title"])); var tags = parsed?["tags"] is JsonArray tagArray ? tagArray.Select(TryReadStringValue) @@ -4221,6 +4220,143 @@ private sealed record SuggestionSamplingResult(string? Title, string[]? Tags); } } + private static string BuildSuggestionSamplingPrompt( + string category, + string? language, + string description, + string? context, + string? toolInvocationContext) + { + var prompt = new StringBuilder(); + var remainingBytes = MaxSamplingPromptBytes; + AppendSamplingPromptLine(prompt, "Extract structured metadata for a cdidx improvement suggestion.", ref remainingBytes); + AppendSamplingPromptLine(prompt, "Return only compact JSON with keys: title (one line, <=80 chars) and tags (array of 1-6 lowercase identifiers).", ref remainingBytes); + AppendSamplingPromptLine(prompt, "Do not include source code.", ref remainingBytes); + AppendSamplingPromptField(prompt, "category", category, MaxSamplingShortFieldChars, ref remainingBytes); + if (!string.IsNullOrWhiteSpace(language)) + AppendSamplingPromptField(prompt, "language", language, MaxSamplingShortFieldChars, ref remainingBytes); + AppendSamplingPromptField(prompt, "description", description, MaxSamplingDescriptionChars, ref remainingBytes); + if (!string.IsNullOrWhiteSpace(context)) + AppendSamplingPromptField(prompt, "context", context, MaxSamplingContextChars, ref remainingBytes); + if (!string.IsNullOrWhiteSpace(toolInvocationContext)) + { + var summary = SummarizeToolInvocationContextForSampling(toolInvocationContext); + AppendSamplingPromptField(prompt, "tool_invocation_context", summary, MaxSamplingToolInvocationSummaryChars, ref remainingBytes); + } + + return prompt.ToString(); + } + + private static void AppendSamplingPromptField(StringBuilder prompt, string name, string value, int maxChars, ref int remainingBytes) + { + var sanitized = SanitizeSamplingPromptField(value, maxChars); + if (sanitized.Length == 0) + return; + AppendSamplingPromptLine(prompt, $"{name}: {sanitized}", ref remainingBytes); + } + + private static void AppendSamplingPromptLine(StringBuilder prompt, string line, ref int remainingBytes) + { + if (remainingBytes <= 0) + return; + + var lineBytes = Encoding.UTF8.GetByteCount(line) + 1; + if (lineBytes > remainingBytes) + { + const string suffix = " ... [truncated]"; + var suffixBytes = Encoding.UTF8.GetByteCount(suffix); + var prefixBudget = remainingBytes - suffixBytes - 1; + if (prefixBudget <= 0) + return; + line = TruncateUtf8(line, prefixBudget).TrimEnd() + suffix; + lineBytes = Encoding.UTF8.GetByteCount(line) + 1; + if (lineBytes > remainingBytes) + return; + } + + prompt.Append(line); + prompt.Append('\n'); + remainingBytes -= lineBytes; + } + + private static string SanitizeSamplingPromptField(string value, int maxChars) + { + var collapsed = CollapseSamplingPromptWhitespace(value); + if (collapsed.Length <= maxChars) + return collapsed; + var end = Math.Min(maxChars, collapsed.Length); + if (end > 0 && char.IsHighSurrogate(collapsed[end - 1])) + end--; + return collapsed[..end].TrimEnd() + " ... [truncated]"; + } + + private static string CollapseSamplingPromptWhitespace(string value) + { + var trimmed = value.Trim(); + var collapsed = new StringBuilder(trimmed.Length); + var previousWhitespace = false; + foreach (var ch in trimmed) + { + if (char.IsControl(ch) || char.IsWhiteSpace(ch)) + { + if (!previousWhitespace) + collapsed.Append(' '); + previousWhitespace = true; + continue; + } + + collapsed.Append(ch); + previousWhitespace = false; + } + + return collapsed.ToString().Trim(); + } + + private static string SummarizeToolInvocationContextForSampling(string value) + { + var trimmed = value.Trim(); + var lineCount = CountLogicalLines(trimmed); + var byteCount = Encoding.UTF8.GetByteCount(trimmed); + return $"provided; {trimmed.Length} chars; {byteCount} UTF-8 bytes; {lineCount} line(s); raw content withheld"; + } + + private static int CountLogicalLines(string value) + { + if (value.Length == 0) + return 0; + var lines = 1; + foreach (var ch in value) + { + if (ch == '\n') + lines++; + } + return lines; + } + + private static string TruncateUtf8(string value, int maxBytes) + { + if (maxBytes <= 0) + return string.Empty; + if (Encoding.UTF8.GetByteCount(value) <= maxBytes) + return value; + + var low = 0; + var high = value.Length; + while (low < high) + { + var mid = low + ((high - low + 1) / 2); + var bytes = Encoding.UTF8.GetByteCount(value.AsSpan(0, mid)); + if (bytes <= maxBytes) + low = mid; + else + high = mid - 1; + } + + if (low > 0 && char.IsHighSurrogate(value[low - 1])) + low--; + return value[..low]; + } + private bool HasClientCapability(string name) => _clientCapabilities is JsonObject obj && obj.TryGetPropertyValue(name, out var node) diff --git a/tests/CodeIndex.Tests/McpServerTests.cs b/tests/CodeIndex.Tests/McpServerTests.cs index 2582fff480..2867910d47 100644 --- a/tests/CodeIndex.Tests/McpServerTests.cs +++ b/tests/CodeIndex.Tests/McpServerTests.cs @@ -9440,6 +9440,154 @@ public void SuggestImprovement_WhenSamplingAvailable_StoresSampledMetadata() Assert.Contains("symbol_extraction", stored.SampledTags!); } + [Fact] + public void SuggestImprovement_WhenSamplingResponseIsTooLarge_IgnoresSampledMetadata() + { + _server.HandleMessage(JsonNode.Parse( + """{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"capabilities":{"sampling":{}}}}""")!); + _server.ClientRequestHandlerForTests = (method, _) => + { + Assert.Equal("sampling/createMessage", method); + return new JsonObject + { + ["content"] = new JsonObject + { + ["type"] = "text", + ["text"] = $$"""{"title":"{{new string('A', 9000)}}","tags":["security"]}""" + } + }; + }; + var uniqueDesc = $"Oversized sampling response regression {Guid.NewGuid():N}"; + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 1, + ["method"] = "tools/call", + ["params"] = new JsonObject + { + ["name"] = "suggest_improvement", + ["arguments"] = new JsonObject + { + ["category"] = "other", + ["description"] = uniqueDesc, + } + } + }; + + var response = _server.HandleMessage(request)!; + + var structured = response["result"]!["structuredContent"]!; + Assert.Equal("recorded", structured["status"]!.GetValue()); + Assert.Null(structured["sampled_title"]); + var stored = new SuggestionStore(Path.GetDirectoryName(_dbPath)!, Path.GetFileNameWithoutExtension(_dbPath)).LoadAll() + .Single(s => s.Description == uniqueDesc); + Assert.Null(stored.SampledTitle); + Assert.Null(stored.SampledTags); + } + + [Fact] + public void SuggestImprovement_WhenSamplingResponseJsonIsTooDeep_IgnoresSampledMetadata() + { + _server.HandleMessage(JsonNode.Parse( + """{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"capabilities":{"sampling":{}}}}""")!); + _server.ClientRequestHandlerForTests = (method, _) => + { + Assert.Equal("sampling/createMessage", method); + var deepTail = new string('[', 40) + "null" + new string(']', 40); + return new JsonObject + { + ["content"] = new JsonObject + { + ["type"] = "text", + ["text"] = $$"""{"title":"Deep sampling metadata","tags":["security"],"nested":{{deepTail}}}""" + } + }; + }; + var uniqueDesc = $"Deep sampling response regression {Guid.NewGuid():N}"; + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 1, + ["method"] = "tools/call", + ["params"] = new JsonObject + { + ["name"] = "suggest_improvement", + ["arguments"] = new JsonObject + { + ["category"] = "other", + ["description"] = uniqueDesc, + } + } + }; + + var response = _server.HandleMessage(request)!; + + var structured = response["result"]!["structuredContent"]!; + Assert.Equal("recorded", structured["status"]!.GetValue()); + Assert.Null(structured["sampled_title"]); + var stored = new SuggestionStore(Path.GetDirectoryName(_dbPath)!, Path.GetFileNameWithoutExtension(_dbPath)).LoadAll() + .Single(s => s.Description == uniqueDesc); + Assert.Null(stored.SampledTitle); + Assert.Null(stored.SampledTags); + } + + [Fact] + public void SuggestImprovement_WhenSamplingAvailable_BoundsPromptAndSummarizesInvocationContext() + { + _server.HandleMessage(JsonNode.Parse( + """{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"capabilities":{"sampling":{}}}}""")!); + string? capturedPrompt = null; + _server.ClientRequestHandlerForTests = (method, parameters) => + { + Assert.Equal("sampling/createMessage", method); + capturedPrompt = parameters?["messages"]?[0]?["content"]?["text"]?.GetValue(); + return new JsonObject + { + ["content"] = new JsonObject + { + ["type"] = "text", + ["text"] = """{"title":"Bound sampling prompt","tags":["security"]}""" + } + }; + }; + var uniqueDesc = new string('\u3042', 2000); + var context = new string('\u3044', 1000); + const string secretValue = "secret-token-1234567890"; + var toolInvocationContext = $"search request included token {secretValue} and detailed invocation payload"; + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 1, + ["method"] = "tools/call", + ["params"] = new JsonObject + { + ["name"] = "suggest_improvement", + ["arguments"] = new JsonObject + { + ["category"] = "other", + ["description"] = uniqueDesc, + ["context"] = context, + ["toolInvocationContext"] = toolInvocationContext, + } + } + }; + + var response = _server.HandleMessage(request)!; + + var structured = response["result"]!["structuredContent"]!; + Assert.Equal("recorded", structured["status"]!.GetValue()); + Assert.Equal("Bound sampling prompt", structured["sampled_title"]!.GetValue()); + Assert.NotNull(capturedPrompt); + Assert.True(Encoding.UTF8.GetByteCount(capturedPrompt) <= 4096); + Assert.Contains("tool_invocation_context: provided;", capturedPrompt); + Assert.Contains("raw content withheld", capturedPrompt); + Assert.DoesNotContain(secretValue, capturedPrompt); + Assert.Contains("[truncated]", capturedPrompt); + var stored = new SuggestionStore(Path.GetDirectoryName(_dbPath)!, Path.GetFileNameWithoutExtension(_dbPath)).LoadAll() + .Single(s => s.Description == uniqueDesc); + Assert.Equal(toolInvocationContext, stored.ToolInvocationContext); + } + [Fact] public void SuggestImprovement_WhenSamplingDisabled_DoesNotCallClientSampling() {