diff --git a/changelog.d/unreleased/3073.fixed.md b/changelog.d/unreleased/3073.fixed.md new file mode 100644 index 0000000000..7c429c5322 --- /dev/null +++ b/changelog.d/unreleased/3073.fixed.md @@ -0,0 +1,16 @@ +--- +category: fixed +issues: + - 3073 +affected: + - src/CodeIndex/Mcp/McpToolHandlers.cs + - tests/CodeIndex.Tests/McpServerTests.cs +--- + +## English + +- **MCP search guard filters now stop at the configured limit (#3073)** - guard filter arrays now track the combined filter budget while they are read and reject the ninth filter immediately instead of materializing the remaining entries first. + +## 日本語 + +- **MCP search guard filter が設定上限で即時停止するようになりました (#3073)** - guard filter 配列は読み取り中に合計filter数の予算を追跡し、残りの要素を materialize する前に9件目を即時拒否するようになりました。 diff --git a/changelog.d/unreleased/3112.fixed.md b/changelog.d/unreleased/3112.fixed.md new file mode 100644 index 0000000000..59d8cbeae0 --- /dev/null +++ b/changelog.d/unreleased/3112.fixed.md @@ -0,0 +1,16 @@ +--- +category: fixed +issues: + - 3112 +affected: + - src/CodeIndex/Mcp/McpServer.cs + - tests/CodeIndex.Tests/McpServerTests.cs +--- + +## English + +- **MCP resources/list now rejects unsafe cursors (#3112)** - `resources/list` now validates cursor offsets against the MCP pagination cap, computes page limits with checked arithmetic, and only returns `nextCursor` values that remain within the accepted cursor range. + +## 日本語 + +- **MCP resources/list が危険な cursor を拒否するようになりました (#3112)** - `resources/list` は cursor offset を MCP pagination 上限で検証し、page limit を checked 演算で計算し、受理可能な範囲内の `nextCursor` だけを返すようになりました。 diff --git a/changelog.d/unreleased/3193.fixed.md b/changelog.d/unreleased/3193.fixed.md new file mode 100644 index 0000000000..04786d190d --- /dev/null +++ b/changelog.d/unreleased/3193.fixed.md @@ -0,0 +1,16 @@ +--- +category: fixed +issues: + - 3193 +affected: + - src/CodeIndex/Mcp/McpToolHandlers.cs + - tests/CodeIndex.Tests/McpServerTests.cs +--- + +## English + +- **MCP search now rejects malformed cursor domains (#3193)** - search cursors now reject non-finite scores and negative chunk or offset fields before pagination continues. + +## 日本語 + +- **MCP search が不正な cursor ドメインを拒否するようになりました (#3193)** - search cursor は pagination を継続する前に、非有限の score と負の chunk / offset フィールドを拒否するようになりました。 diff --git a/src/CodeIndex/Mcp/McpServer.cs b/src/CodeIndex/Mcp/McpServer.cs index fd51f8ad28..de96cb13b3 100644 --- a/src/CodeIndex/Mcp/McpServer.cs +++ b/src/CodeIndex/Mcp/McpServer.cs @@ -1881,17 +1881,31 @@ private JsonNode HandleResourcesList(JsonNode? id, JsonNode? listParams) { const int pageSize = 200; var offset = 0; - if (listParams?["cursor"] is JsonValue cursorValue - && cursorValue.TryGetValue(out var cursor) - && int.TryParse(cursor, System.Globalization.NumberStyles.None, System.Globalization.CultureInfo.InvariantCulture, out var parsed) - && parsed > 0) + if (listParams?["cursor"] is JsonNode cursorNode) { - offset = parsed; + if (cursorNode is not JsonValue cursorValue + || !cursorValue.TryGetValue(out var cursor) + || !int.TryParse(cursor, NumberStyles.None, CultureInfo.InvariantCulture, out offset) + || offset < 0 + || offset > MaxMcpPaginationOffset) + { + return CreateResourcesListCursorError(id); + } + } + + int listLimit; + try + { + listLimit = checked(offset + pageSize + 1); + } + catch (OverflowException) + { + return CreateResourcesListCursorError(id); } return WithDbReader(id, args: null, reader => { - var files = reader.ListFiles(limit: offset + pageSize + 1); + var files = reader.ListFiles(limit: listLimit); var page = files.Skip(offset).Take(pageSize).ToArray(); var resources = new JsonArray(); foreach (var file in page) @@ -1913,12 +1927,24 @@ private JsonNode HandleResourcesList(JsonNode? id, JsonNode? listParams) { ["resources"] = resources, }; - if (offset + pageSize < files.Count) - result["nextCursor"] = (offset + pageSize).ToString(System.Globalization.CultureInfo.InvariantCulture); + var nextOffset = offset + pageSize; + if (nextOffset <= MaxMcpPaginationOffset && nextOffset < files.Count) + result["nextCursor"] = nextOffset.ToString(CultureInfo.InvariantCulture); return CreateSuccessResponse(true, id, result); }); } + private static JsonObject CreateResourcesListCursorError(JsonNode? id) + => CreateErrorResponse(hasId: true, id: id, code: -32602, + message: $"resources/list cursor must be a non-negative pagination offset no greater than {MaxMcpPaginationOffset}.", + category: McpErrorEnvelope.CategoryInvalidArgument, + suggestion: "Use the `nextCursor` value returned by the previous resources/list response, or omit params.cursor to start from the first page.", + retrySafe: false, + extraData: new JsonObject + { + ["max_pagination_offset"] = MaxMcpPaginationOffset, + }); + private JsonNode HandleResourcesRead(JsonNode? id, JsonNode? readParams) { var uri = TryReadStringValue(readParams?["uri"]); diff --git a/src/CodeIndex/Mcp/McpToolHandlers.cs b/src/CodeIndex/Mcp/McpToolHandlers.cs index 8750eb0da1..372edadd88 100644 --- a/src/CodeIndex/Mcp/McpToolHandlers.cs +++ b/src/CodeIndex/Mcp/McpToolHandlers.cs @@ -189,9 +189,11 @@ private static bool TryParseSearchCursor(string value, out SearchCursor cursor) if (firstSeparator <= 0 || firstSeparator == lastSeparator - 1) return false; - if (!double.TryParse(value.AsSpan(0, firstSeparator), NumberStyles.Float, CultureInfo.InvariantCulture, out var score)) + if (!double.TryParse(value.AsSpan(0, firstSeparator), NumberStyles.Float, CultureInfo.InvariantCulture, out var score) + || !double.IsFinite(score)) return false; - if (!long.TryParse(value.AsSpan(firstSeparator + 1, lastSeparator - firstSeparator - 1), NumberStyles.None, CultureInfo.InvariantCulture, out var chunkId)) + if (!long.TryParse(value.AsSpan(firstSeparator + 1, lastSeparator - firstSeparator - 1), NumberStyles.None, CultureInfo.InvariantCulture, out var chunkId) + || chunkId < 0) return false; if (!int.TryParse(value.AsSpan(lastSeparator + 1), NumberStyles.None, CultureInfo.InvariantCulture, out var offset) || offset < 0) return false; @@ -418,54 +420,47 @@ private static List ReadStringList(JsonNode? args, string propertyName) : []; } - private JsonNode? TryReadStringOrStringList(JsonNode? id, JsonNode? args, string propertyName, out List values) + private JsonNode? TryReadSearchGuardFilters(JsonNode? id, JsonNode? args, out List filters) { - values = []; - var node = args?[propertyName]; - if (node is null) - return null; + filters = []; + var collected = new List(); - if (node is JsonValue singleValue && singleValue.TryGetValue(out var singleText)) + JsonNode? AddFilter(string propertyName, SearchGuardRole role, SearchGuardDirection direction, string value) { - values.Add(singleText); - return null; - } + if (collected.Count >= DbReader.MaxSearchGuardFilters) + return CreateToolErrorResponse(id, $"search accepts at most {DbReader.MaxSearchGuardFilters} guard filters; got {collected.Count + 1}."); - if (node is JsonArray array) - { - foreach (var item in array) - { - if (item is not JsonValue value || !value.TryGetValue(out var text)) - return CreateToolErrorResponse(id, $"'{propertyName}' entries must be strings."); - values.Add(text); - } + if (string.IsNullOrWhiteSpace(value)) + return CreateToolErrorResponse(id, $"'{propertyName}' entries must be non-empty strings."); + if (value.Length > QueryLimits.MaxQueryLength) + return CreateToolErrorResponse(id, $"'{propertyName}' query too long (max {QueryLimits.MaxQueryLength} characters)."); + + collected.Add(new SearchGuardFilter(role, direction, value)); return null; } - return CreateToolErrorResponse(id, $"'{propertyName}' must be a string or string array."); - } - - private JsonNode? TryReadSearchGuardFilters(JsonNode? id, JsonNode? args, out List filters) - { - filters = []; - var collected = new List(); - JsonNode? AddFilters(string propertyName, SearchGuardRole role, SearchGuardDirection direction) { - if (TryReadStringOrStringList(id, args, propertyName, out var values) is JsonNode readError) - return readError; + var node = args?[propertyName]; + if (node is null) + return null; - foreach (var value in values) - { - if (string.IsNullOrWhiteSpace(value)) - return CreateToolErrorResponse(id, $"'{propertyName}' entries must be non-empty strings."); - if (value.Length > QueryLimits.MaxQueryLength) - return CreateToolErrorResponse(id, $"'{propertyName}' query too long (max {QueryLimits.MaxQueryLength} characters)."); + if (node is JsonValue singleValue && singleValue.TryGetValue(out var singleText)) + return AddFilter(propertyName, role, direction, singleText); - collected.Add(new SearchGuardFilter(role, direction, value)); + if (node is JsonArray array) + { + foreach (var item in array) + { + if (item is not JsonValue value || !value.TryGetValue(out var text)) + return CreateToolErrorResponse(id, $"'{propertyName}' entries must be strings."); + if (AddFilter(propertyName, role, direction, text) is JsonNode addError) + return addError; + } + return null; } - return null; + return CreateToolErrorResponse(id, $"'{propertyName}' must be a string or string array."); } if (AddFilters("requireBefore", SearchGuardRole.Require, SearchGuardDirection.Before) is JsonNode requireBeforeError) @@ -478,9 +473,7 @@ private static List ReadStringList(JsonNode? args, string propertyName) return rejectAfterError; filters = collected; - return filters.Count > DbReader.MaxSearchGuardFilters - ? CreateToolErrorResponse(id, $"search accepts at most {DbReader.MaxSearchGuardFilters} guard filters; got {filters.Count}.") - : null; + return null; } private static JsonObject? ValidateCommonListArguments(JsonNode? args) diff --git a/tests/CodeIndex.Tests/McpServerTests.cs b/tests/CodeIndex.Tests/McpServerTests.cs index 6ba8999fdd..226491f0fe 100644 --- a/tests/CodeIndex.Tests/McpServerTests.cs +++ b/tests/CodeIndex.Tests/McpServerTests.cs @@ -297,6 +297,37 @@ public void ToolsCall_SearchReturnsStableAtAndCursorContinuesAfterAnchor_Issue14 second["results"]!.AsArray()[0]!["path"]!.GetValue()); } + [Theory] + [InlineData("NaN:1:0")] + [InlineData("Infinity:1:0")] + [InlineData("1:-1:0")] + [InlineData("1:1:-1")] + public void ToolsCall_Search_InvalidCursorDomain_ReturnsInvalidCursorError_Issue3193(string cursor) + { + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 1, + ["method"] = "tools/call", + ["params"] = new JsonObject + { + ["name"] = "search", + ["arguments"] = new JsonObject + { + ["query"] = "Run", + ["cursor"] = cursor, + }, + }, + }; + + var response = _server.HandleMessage(request)!; + + var result = response["result"]!; + Assert.True(result["isError"]!.GetValue()); + Assert.Contains("'cursor' must be a search pagination cursor", result["content"]![0]!["text"]!.GetValue()); + Assert.Equal("invalid_argument", result["structuredContent"]!["category"]!.GetValue()); + } + [Fact] public void ToolsCall_Callers_TruncatedResponseIncludesNextOffsetAndPages() { @@ -932,6 +963,86 @@ public void ResourcesList_ReturnsIndexedFilesAsResources() Assert.Equal("text/x-csharp", resource["mimeType"]!.GetValue()); } + [Theory] + [InlineData("-1")] + [InlineData("not-a-cursor")] + public void ResourcesList_InvalidCursor_ReturnsInvalidParams_Issue3112(string cursor) + { + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 1, + ["method"] = "resources/list", + ["params"] = new JsonObject + { + ["cursor"] = cursor, + }, + }; + + var response = _server.HandleMessage(request)!; + + Assert.Equal(-32602, response["error"]!["code"]!.GetValue()); + var data = response["error"]!["data"]!; + Assert.Equal("invalid_argument", data["category"]!.GetValue()); + Assert.Equal(McpServer.MaxMcpPaginationOffset, data["max_pagination_offset"]!.GetValue()); + } + + [Fact] + public void ResourcesList_CursorBeyondPaginationCap_ReturnsInvalidParams_Issue3112() + { + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 1, + ["method"] = "resources/list", + ["params"] = new JsonObject + { + ["cursor"] = (McpServer.MaxMcpPaginationOffset + 1).ToString(System.Globalization.CultureInfo.InvariantCulture), + }, + }; + + var response = _server.HandleMessage(request)!; + + Assert.Equal(-32602, response["error"]!["code"]!.GetValue()); + var data = response["error"]!["data"]!; + Assert.Equal("invalid_argument", data["category"]!.GetValue()); + Assert.Equal(McpServer.MaxMcpPaginationOffset, data["max_pagination_offset"]!.GetValue()); + } + + [Fact] + public void ResourcesList_AtPaginationCap_DoesNotEmitSelfInvalidNextCursor_Issue3112() + { + var writer = new DbWriter(_db.Connection); + using var transaction = writer.BeginTransaction(); + for (var i = 0; i < McpServer.MaxMcpPaginationOffset + 200; i++) + { + writer.UpsertFile(new FileRecord + { + Path = $"zz/paged-{i:D5}.cs", + Lang = "csharp", + Size = 1, + Lines = 1, + Modified = ManualTimeProvider.FixtureUtcNow.UtcDateTime, + Checksum = $"bulk-{i}", + }); + } + transaction.Commit(); + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 1, + ["method"] = "resources/list", + ["params"] = new JsonObject + { + ["cursor"] = McpServer.MaxMcpPaginationOffset.ToString(System.Globalization.CultureInfo.InvariantCulture), + }, + }; + + var response = _server.HandleMessage(request)!; + + Assert.Null(response["result"]!["nextCursor"]); + } + [Fact] public void ResourcesList_DoesNotAdvertiseUrisTooLongToRead_Issue3122() { @@ -4033,6 +4144,42 @@ public void NonAtomic(string path) Assert.Equal("File.Move", evidence["query"]!.GetValue()); } + [Fact] + public void ToolsCall_Search_GuardFiltersFailFastWhenCombinedArraysExceedLimit_Issue3073() + { + var requireBefore = new JsonArray(); + for (var i = 0; i < DbReader.MaxSearchGuardFilters; i++) + requireBefore.Add($"Guard{i}"); + var requireAfter = new JsonArray(); + requireAfter.Add("Overflow"); + requireAfter.Add(42); + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 1, + ["method"] = "tools/call", + ["params"] = new JsonObject + { + ["name"] = "search", + ["arguments"] = new JsonObject + { + ["query"] = "Run", + ["requireBefore"] = requireBefore, + ["requireAfter"] = requireAfter, + }, + }, + }; + + var response = _server.HandleMessage(request)!; + + var result = response["result"]!; + Assert.True(result["isError"]!.GetValue()); + var text = result["content"]![0]!["text"]!.GetValue(); + Assert.Contains($"search accepts at most {DbReader.MaxSearchGuardFilters} guard filters; got {DbReader.MaxSearchGuardFilters + 1}.", text); + Assert.DoesNotContain("entries must be strings", text); + Assert.Equal("invalid_argument", result["structuredContent"]!["category"]!.GetValue()); + } + [Fact] public void ToolsCall_Search_GuardPaginationResumesWithinSplitChunk_Issue2852() {