diff --git a/changelog.d/unreleased/3160.fixed.md b/changelog.d/unreleased/3160.fixed.md new file mode 100644 index 0000000000..13224f7cf9 --- /dev/null +++ b/changelog.d/unreleased/3160.fixed.md @@ -0,0 +1,17 @@ +--- +category: fixed +issues: + - 3160 +affected: + - src/CodeIndex/Mcp/McpServer.cs + - src/CodeIndex/Mcp/McpToolHandlers.cs + - tests/CodeIndex.Tests/McpServerTests.cs +--- + +## English + +- **MCP project filter resolver failures now return structured invalid-argument errors (#3160)** — unresolved or inaccessible project filters now fail before tool execution with a bounded diagnostic instead of falling through to a generic sanitized tool failure. + +## 日本語 + +- **MCP project filter resolver failure が structured invalid-argument error を返すようになりました (#3160)** — 解決できない、またはアクセスできない project filter は tool 実行前に bounded diagnostic 付きで失敗し、generic な sanitized tool failure に落ちないようになりました。 diff --git a/changelog.d/unreleased/3182.fixed.md b/changelog.d/unreleased/3182.fixed.md new file mode 100644 index 0000000000..0d3ef7f36b --- /dev/null +++ b/changelog.d/unreleased/3182.fixed.md @@ -0,0 +1,17 @@ +--- +category: fixed +issues: + - 3182 +affected: + - src/CodeIndex/Mcp/McpToolDefinitions.cs + - src/CodeIndex/Mcp/McpToolHandlers.cs + - tests/CodeIndex.Tests/McpServerTests.cs +--- + +## English + +- **MCP path/list argument validation now shares CLI path-filter bounds (#3182)** — path-like arrays now use the CLI count and length limits, and `tools/list` advertises those bounds for client-side validation. + +## 日本語 + +- **MCP path/list 引数検証が CLI の path-filter 上限を共有するようになりました (#3182)** — path 系配列は CLI と同じ件数・長さ上限を使い、`tools/list` でもクライアント側検証用にその上限を広告します。 diff --git a/changelog.d/unreleased/3184.fixed.md b/changelog.d/unreleased/3184.fixed.md new file mode 100644 index 0000000000..39122e6a67 --- /dev/null +++ b/changelog.d/unreleased/3184.fixed.md @@ -0,0 +1,16 @@ +--- +category: fixed +issues: + - 3184 +affected: + - src/CodeIndex/Mcp/McpToolHandlers.cs + - tests/CodeIndex.Tests/McpServerTests.cs +--- + +## English + +- **MCP `impact_analysis` now enforces the shared query length limit (#3184)** — oversized impact queries now fail with the same structured invalid-argument response used by other query tools before impact analysis runs. + +## 日本語 + +- **MCP `impact_analysis` が共有 query 長上限を適用するようになりました (#3184)** — 長すぎる impact query は impact analysis 実行前に、他の query tool と同じ structured invalid-argument response で失敗します。 diff --git a/changelog.d/unreleased/3186.fixed.md b/changelog.d/unreleased/3186.fixed.md new file mode 100644 index 0000000000..0a54e3cd07 --- /dev/null +++ b/changelog.d/unreleased/3186.fixed.md @@ -0,0 +1,17 @@ +--- +category: fixed +issues: + - 3186 +affected: + - src/CodeIndex/Mcp/McpToolDefinitions.cs + - src/CodeIndex/Mcp/McpToolHandlers.cs + - tests/CodeIndex.Tests/McpServerTests.cs +--- + +## English + +- **MCP required `path` arguments now fail before lookup when invalid (#3186)** — `outline` and `excerpt` now reject invalid indexed-file paths before database lookup, while `index` rejects non-string, overlong, or NUL-containing project paths before filesystem checks. + +## 日本語 + +- **MCP の必須 `path` 引数が不正な場合 lookup 前に失敗するようになりました (#3186)** — `outline` と `excerpt` は不正な indexed-file path を database lookup 前に拒否し、`index` は non-string、長すぎる値、NUL を含む project path を filesystem check 前に拒否します。 diff --git a/changelog.d/unreleased/3192.fixed.md b/changelog.d/unreleased/3192.fixed.md new file mode 100644 index 0000000000..4911eb4e60 --- /dev/null +++ b/changelog.d/unreleased/3192.fixed.md @@ -0,0 +1,15 @@ +--- +category: fixed +issues: + - 3192 +affected: + - tests/CodeIndex.Tests/McpToolContractTests.cs +--- + +## English + +- **MCP `search.cursor` contract is now regression-locked (#3192)** — `cursor` stays advertised, allowlisted, and string-typed across `tools/list` and server-side argument validation. + +## 日本語 + +- **MCP `search.cursor` contract を regression lock しました (#3192)** — `cursor` が `tools/list` と server-side argument validation の両方で advertised / allowlisted / string-typed のまま保たれるよう確認します。 diff --git a/changelog.d/unreleased/3194.fixed.md b/changelog.d/unreleased/3194.fixed.md new file mode 100644 index 0000000000..1539f14ea7 --- /dev/null +++ b/changelog.d/unreleased/3194.fixed.md @@ -0,0 +1,16 @@ +--- +category: fixed +issues: + - 3194 +affected: + - src/CodeIndex/Mcp/McpToolHandlers.cs + - tests/CodeIndex.Tests/McpServerTests.cs +--- + +## English + +- **MCP `definition` and `symbols` now reject invalid `since` values (#3194)** — invalid ISO 8601 timestamps now return the same structured invalid-argument response used by other MCP tools instead of silently dropping the freshness filter. + +## 日本語 + +- **MCP `definition` / `symbols` が不正な `since` 値を拒否するようになりました (#3194)** — 不正な ISO 8601 timestamp は freshness filter を黙って無視せず、他の MCP tool と同じ structured invalid-argument response を返します。 diff --git a/changelog.d/unreleased/3195.fixed.md b/changelog.d/unreleased/3195.fixed.md new file mode 100644 index 0000000000..46b133e31c --- /dev/null +++ b/changelog.d/unreleased/3195.fixed.md @@ -0,0 +1,16 @@ +--- +category: fixed +issues: + - 3195 +affected: + - src/CodeIndex/Mcp/McpToolHandlers.cs + - tests/CodeIndex.Tests/McpServerTests.cs +--- + +## English + +- **MCP pagination now rejects invalid lower bounds (#3195)** — non-positive `limit` values and negative `offset` values now return structured invalid-params errors instead of being silently clamped. + +## 日本語 + +- **MCP pagination が不正な下限値を拒否するようになりました (#3195)** — 非正の `limit` と負の `offset` は、黙って補正されず structured invalid-params error を返すようになりました。 diff --git a/changelog.d/unreleased/3196.fixed.md b/changelog.d/unreleased/3196.fixed.md new file mode 100644 index 0000000000..f8dd57b2c1 --- /dev/null +++ b/changelog.d/unreleased/3196.fixed.md @@ -0,0 +1,15 @@ +--- +category: fixed +issues: + - 3196 +affected: + - tests/CodeIndex.Tests/McpToolContractTests.cs +--- + +## English + +- **MCP `deps` argument contract is now regression-locked (#3196)** — `reverse`, `format`, and `cycles` stay advertised, allowlisted, and typed, while stale hidden arguments such as `direction` and `includeGenerated` stay absent. + +## 日本語 + +- **MCP `deps` argument contract を regression lock しました (#3196)** — `reverse`、`format`、`cycles` が advertised / allowlisted / typed のまま保たれ、`direction` や `includeGenerated` のような古い hidden 引数が戻らないことを確認します。 diff --git a/changelog.d/unreleased/3197.fixed.md b/changelog.d/unreleased/3197.fixed.md new file mode 100644 index 0000000000..01d802eaee --- /dev/null +++ b/changelog.d/unreleased/3197.fixed.md @@ -0,0 +1,15 @@ +--- +category: fixed +issues: + - 3197 +affected: + - tests/CodeIndex.Tests/McpToolContractTests.cs +--- + +## English + +- **MCP `map` argument contract is now regression-locked (#3197)** — `sections` and `depth` stay advertised, allowlisted, and validated with their documented JSON shapes. + +## 日本語 + +- **MCP `map` argument contract を regression lock しました (#3197)** — `sections` と `depth` が advertised / allowlisted され、documented JSON shape のまま検証されることを確認します。 diff --git a/changelog.d/unreleased/3198.fixed.md b/changelog.d/unreleased/3198.fixed.md new file mode 100644 index 0000000000..8cbf913a28 --- /dev/null +++ b/changelog.d/unreleased/3198.fixed.md @@ -0,0 +1,15 @@ +--- +category: fixed +issues: + - 3198 +affected: + - tests/CodeIndex.Tests/McpToolContractTests.cs +--- + +## English + +- **MCP `outline` and `validate` no-op argument contracts are now regression-locked (#3198)** — hidden no-op arguments stay absent and advertised arguments stay aligned with server-side allowlists. + +## 日本語 + +- **MCP `outline` / `validate` の no-op 引数 contract を regression lock しました (#3198)** — hidden no-op 引数が戻らず、advertised 引数と server-side allowlist が一致し続けることを確認します。 diff --git a/src/CodeIndex/Mcp/McpServer.cs b/src/CodeIndex/Mcp/McpServer.cs index 5d1ddfccbd..aa39522dfb 100644 --- a/src/CodeIndex/Mcp/McpServer.cs +++ b/src/CodeIndex/Mcp/McpServer.cs @@ -2479,6 +2479,15 @@ JsonObject CreateUnknownToolResponseForMetrics() DeferFrameLog(BuildRateLimitedLog(toolName, _caller, decision.RetryAfterMs)); response = CreateRateLimitedErrorResponse(id, toolName, _caller, decision.RetryAfterMs); } + else if (ValidateProjectFilterArguments(args) is JsonObject projectFilterError) + { + metricsError = "invalid_project_filter"; + response = CreateToolErrorResponse(id, projectFilterError["message"]!.GetValue(), + category: McpErrorEnvelope.CategoryInvalidArgument, + suggestion: "Use a project name or project path from the current workspace, or correct the solution filter.", + retrySafe: false, + extraData: projectFilterError); + } else { response = toolName switch diff --git a/src/CodeIndex/Mcp/McpToolDefinitions.cs b/src/CodeIndex/Mcp/McpToolDefinitions.cs index 2ee00a335e..31958f131e 100644 --- a/src/CodeIndex/Mcp/McpToolDefinitions.cs +++ b/src/CodeIndex/Mcp/McpToolDefinitions.cs @@ -643,13 +643,48 @@ private static void ApplyCommonSchemaConstraint(string toolName, string name, Js case "path": case "project": case "solution": - obj.TryAdd("minLength", 1); - obj.TryAdd("maxLength", 4096); - obj.TryAdd("pattern", @"^(?!/)(?![A-Za-z]:)(?!.*(^|/)\.\.(/|$))(?!.*\u0000).*$"); - AppendConstraintDescription(obj, "Must be workspace-relative, non-empty, and must not contain NUL bytes or `..` path traversal segments."); + if (obj["type"]?.GetValue() == "array") + { + obj.TryAdd("maxItems", MaxMcpArrayFilterCount); + } + else if (name == "path" && toolName == "index") + { + obj.TryAdd("minLength", 1); + obj.TryAdd("maxLength", MaxMcpArrayFilterStringLength); + obj.TryAdd("pattern", @"^(?!.*\u0000).+$"); + AppendConstraintDescription(obj, "May be absolute or relative, but must be non-empty and must not contain NUL bytes."); + } + else + { + obj.TryAdd("minLength", 1); + obj.TryAdd("maxLength", MaxMcpArrayFilterStringLength); + obj.TryAdd("pattern", @"^(?!/)(?![A-Za-z]:)(?!.*(^|/)\.\.(/|$))(?!.*\u0000).*$"); + AppendConstraintDescription(obj, "Must be workspace-relative, non-empty, and must not contain NUL bytes or `..` path traversal segments."); + } break; case "excludePaths": - obj.TryAdd("maxItems", 100); + if (obj["type"]?.GetValue() == "array") + { + obj.TryAdd("maxItems", MaxMcpArrayFilterCount); + } + else + { + obj.TryAdd("minLength", 1); + obj.TryAdd("maxLength", MaxMcpArrayFilterStringLength); + obj.TryAdd("pattern", @"^(?!/)(?![A-Za-z]:)(?!.*(^|/)\.\.(/|$))(?!.*\u0000).*$"); + AppendConstraintDescription(obj, "Must be workspace-relative, non-empty, and must not contain NUL bytes or `..` path traversal segments."); + } + break; + case "sections": + if (obj["type"]?.GetValue() == "array") + { + obj.TryAdd("maxItems", MaxMcpArrayFilterCount); + } + else + { + obj.TryAdd("minLength", 1); + obj.TryAdd("maxLength", MaxMcpArrayFilterStringLength); + } break; case "limit": obj.TryAdd("minimum", 1); diff --git a/src/CodeIndex/Mcp/McpToolHandlers.cs b/src/CodeIndex/Mcp/McpToolHandlers.cs index d95aacb6d6..03bff5cf22 100644 --- a/src/CodeIndex/Mcp/McpToolHandlers.cs +++ b/src/CodeIndex/Mcp/McpToolHandlers.cs @@ -23,8 +23,8 @@ public partial class McpServer internal const int MaxBatchQueryResponseByteLimit = 10 * 1024 * 1024; private const int DefaultExcerptOutputByteLimit = MaxLineByteLength; private const string BatchQueryResponseByteLimitEnvVar = "CDIDX_MCP_BATCH_RESPONSE_MAX_BYTES"; - internal const int MaxMcpArrayFilterCount = 100; - internal const int MaxMcpArrayFilterStringLength = 4096; + internal const int MaxMcpArrayFilterCount = QueryCommandRunner.MaxQueryPathFilterCount; + internal const int MaxMcpArrayFilterStringLength = QueryCommandRunner.MaxQueryPathFilterLength; private static readonly HashSet BoundedEnumLikeScalarArguments = new(StringComparer.Ordinal) { "category", @@ -536,12 +536,40 @@ private static List ReadStringList(JsonNode? args, string propertyName) if (ValidateToolArgumentTypes(toolName, obj) is JsonObject typeError) return typeError; + if (ValidateToolArgumentRanges(toolName, obj) is JsonObject rangeError) + return rangeError; + if (ValidateBoundedEnumLikeScalarArguments(toolName, obj) is JsonObject scalarError) return scalarError; return null; } + private static JsonObject? ValidateToolArgumentRanges(string toolName, JsonObject args) + { + if (args["limit"] is JsonValue limitValue + && limitValue.TryGetValue(out var limit) + && limit <= 0) + return CreateIntegerMinimumArgumentError(toolName, "limit", minimum: 1, actual: limit); + + if (args["offset"] is JsonValue offsetValue + && offsetValue.TryGetValue(out var offset) + && offset < 0) + return CreateIntegerMinimumArgumentError(toolName, "offset", minimum: 0, actual: offset); + + return null; + } + + private static JsonObject CreateIntegerMinimumArgumentError(string toolName, string argumentName, int minimum, int actual) => new() + { + ["message"] = $"Argument '{argumentName}' on tool '{toolName}' must be greater than or equal to {minimum}; got {actual}.", + ["tool"] = toolName, + ["parameter"] = argumentName, + ["minimum"] = minimum, + ["actual"] = actual, + ["jsonrpc_invalid_params"] = true, + }; + private static JsonObject? ValidateBoundedEnumLikeScalarArguments(string toolName, JsonObject args) { foreach (var property in args) @@ -999,6 +1027,54 @@ private JsonObject ToAnalyzeSymbolJsonObject(SymbolAnalysisResult analysis) return paths.Count == 0 ? null : paths; } + private static JsonObject? ValidateProjectFilterArguments(JsonNode? args) + { + var projects = ReadPathList(args, "project") ?? []; + if (projects.Count == 0) + return null; + + var solution = args?["solution"]?.GetValue(); + try + { + _ = SolutionProjectResolver.ResolveProjectDirectoryGlobs(Environment.CurrentDirectory, projects, solution); + return null; + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or InvalidOperationException) + { + var diagnostic = McpBoundedText.ForDisplay(ex.Message); + var error = new JsonObject + { + ["message"] = $"Project filter could not be resolved: {diagnostic.Text}", + ["parameter"] = "project", + ["diagnostic"] = diagnostic.Text, + }; + diagnostic.AddMetadata(error, "diagnostic"); + return error; + } + } + + private static bool TryReadSinceArgument(JsonNode? args, out DateTime? since, out string? error) + { + var sinceStr = args?["since"]?.GetValue(); + if (sinceStr == null) + { + since = null; + error = null; + return true; + } + + if (QueryCommandRunner.TryParseIso8601Since(sinceStr, out var parsedSince)) + { + since = parsedSince; + error = null; + return true; + } + + since = null; + error = $"Invalid 'since' timestamp: '{sinceStr}'. Use ISO 8601 format (e.g. 2024-01-01 or 2024-01-01T00:00:00Z)."; + return false; + } + private static bool TryReadRequiredStringParameter(JsonNode? args, string propertyName, out string value, out string? error) { var node = args?[propertyName]; @@ -1020,6 +1096,57 @@ private static bool TryReadRequiredStringParameter(JsonNode? args, string proper return true; } + private static bool TryReadRequiredPathParameter(JsonNode? args, string propertyName, out string value, out string? error) + { + if (!TryReadRequiredStringParameter(args, propertyName, out value, out error)) + return false; + + if (value.Length > MaxMcpArrayFilterStringLength) + { + error = $"Parameter \"{propertyName}\" must be no longer than {MaxMcpArrayFilterStringLength} characters."; + return false; + } + + var normalized = value.Replace("\\", "/", StringComparison.Ordinal); + if (value.IndexOf("\0", StringComparison.Ordinal) >= 0 + || normalized.StartsWith("/", StringComparison.Ordinal) + || HasWindowsDrivePrefix(normalized) + || normalized.Split(new[] { '/' }, StringSplitOptions.None).Any(segment => segment == "..")) + { + error = $"Parameter \"{propertyName}\" must be workspace-relative and must not contain NUL bytes or `..` path traversal segments."; + return false; + } + + error = null; + return true; + } + + private static bool TryReadRequiredIndexPathParameter(JsonNode? args, string propertyName, out string value, out string? error) + { + if (!TryReadRequiredStringParameter(args, propertyName, out value, out error)) + return false; + + if (value.Length > MaxMcpArrayFilterStringLength) + { + error = $"Parameter \"{propertyName}\" must be no longer than {MaxMcpArrayFilterStringLength} characters."; + return false; + } + + if (value.IndexOf("\0", StringComparison.Ordinal) >= 0) + { + error = $"Parameter \"{propertyName}\" must not contain NUL bytes."; + return false; + } + + error = null; + return true; + } + + private static bool HasWindowsDrivePrefix(string path) + => path.Length >= 2 + && path[1] == ':' + && ((path[0] >= 'A' && path[0] <= 'Z') || (path[0] >= 'a' && path[0] <= 'z')); + private static bool HasBlankPathFilter(JsonNode? args) { var node = args?["path"]; @@ -1119,15 +1246,8 @@ private JsonNode ExecuteSearch(JsonNode? id, JsonNode? args) var pathPatterns = ReadScopedPathList(args); var excludePaths = ReadStringList(args, "excludePaths"); var excludeTests = args?["excludeTests"]?.GetValue() ?? false; - var sinceStr = args?["since"]?.GetValue(); - DateTime? since = null; - if (sinceStr != null) - { - if (QueryCommandRunner.TryParseIso8601Since(sinceStr, out var parsedSince)) - since = parsedSince; - else - return CreateToolErrorResponse(id, $"Invalid 'since' timestamp: '{sinceStr}'. Use ISO 8601 format (e.g. 2024-01-01 or 2024-01-01T00:00:00Z)."); - } + if (!TryReadSinceArgument(args, out var since, out var sinceError)) + return CreateToolErrorResponse(id, sinceError!); var deduplicate = !(args?["noDedup"]?.GetValue() ?? false); var format = ReadResponseFormat(args); if (ValidateResponseFormat(format) is string formatError) @@ -1278,10 +1398,8 @@ private JsonNode ExecuteSymbols(JsonNode? id, JsonNode? args) var pathPatterns = ReadScopedPathList(args); var excludePaths = ReadStringList(args, "excludePaths"); var excludeTests = args?["excludeTests"]?.GetValue() ?? false; - var sinceStr = args?["since"]?.GetValue(); - DateTime? since = null; - if (sinceStr != null && QueryCommandRunner.TryParseIso8601Since(sinceStr, out var parsedSince)) - since = parsedSince; + if (!TryReadSinceArgument(args, out var since, out var sinceError)) + return CreateToolErrorResponse(id, sinceError!); if (!TryResolveNameExactArgument(args, "symbols", out var exact, out var exactError)) return CreateToolErrorResponse(id, exactError!); @@ -1373,10 +1491,8 @@ private JsonNode ExecuteDefinition(JsonNode? id, JsonNode? args) var pathPatterns = ReadScopedPathList(args); var excludePaths = ReadStringList(args, "excludePaths"); var excludeTests = args?["excludeTests"]?.GetValue() ?? false; - var sinceStr = args?["since"]?.GetValue(); - DateTime? since = null; - if (sinceStr != null && QueryCommandRunner.TryParseIso8601Since(sinceStr, out var parsedDefSince)) - since = parsedDefSince; + if (!TryReadSinceArgument(args, out var since, out var sinceError)) + return CreateToolErrorResponse(id, sinceError!); if (!TryResolveNameExactArgument(args, "definition", out var exact, out var exactError)) return CreateToolErrorResponse(id, exactError!); var format = ReadResponseFormat(args); @@ -1738,15 +1854,8 @@ private JsonNode ExecuteFiles(JsonNode? id, JsonNode? args) var pathPatterns = ReadScopedPathList(args); var excludePaths = ReadStringList(args, "excludePaths"); var excludeTests = args?["excludeTests"]?.GetValue() ?? false; - var sinceStr = args?["since"]?.GetValue(); - DateTime? since = null; - if (sinceStr != null) - { - if (QueryCommandRunner.TryParseIso8601Since(sinceStr, out var parsedSince)) - since = parsedSince; - else - return CreateToolErrorResponse(id, $"Invalid 'since' timestamp: '{sinceStr}'. Use ISO 8601 format (e.g. 2024-01-01 or 2024-01-01T00:00:00Z)."); - } + if (!TryReadSinceArgument(args, out var since, out var sinceError)) + return CreateToolErrorResponse(id, sinceError!); return WithDbReader(id, args, reader => { @@ -2243,7 +2352,7 @@ private static string QuoteCommandArgument(string value) private JsonNode ExecuteOutline(JsonNode? id, JsonNode? args) { - if (!TryReadRequiredStringParameter(args, "path", out var path, out var requiredError)) + if (!TryReadRequiredPathParameter(args, "path", out var path, out var requiredError)) return CreateToolErrorResponse(id, requiredError!); return WithDbReader(id, args, reader => @@ -2267,7 +2376,7 @@ private JsonNode ExecuteOutline(JsonNode? id, JsonNode? args) private JsonNode ExecuteExcerpt(JsonNode? id, JsonNode? args) { - if (!TryReadRequiredStringParameter(args, "path", out var path, out var requiredError)) + if (!TryReadRequiredPathParameter(args, "path", out var path, out var requiredError)) return CreateToolErrorResponse(id, requiredError!); var startLine = args?["startLine"]?.GetValue(); @@ -2785,6 +2894,16 @@ void AppendRateLimitedSlot(int requestIndex, string? toolName, JsonNode? toolArg continue; } + if (ValidateProjectFilterArguments(toolArgs) is JsonObject projectFilterError) + { + AppendSlotError(requestIndex, toolName, toolArgs, slotStopwatch, projectFilterError["message"]!.GetValue(), + category: McpErrorEnvelope.CategoryInvalidArgument, + suggestion: "Use a project name or project path from the current workspace, or correct the solution filter.", + retrySafe: false, + extraData: projectFilterError); + continue; + } + try { // Execute the tool and extract the structured content / ツールを実行し構造化コンテンツを抽出 @@ -3113,6 +3232,8 @@ private JsonNode ExecuteImpactAnalysis(JsonNode? id, JsonNode? args) { if (!TryReadRequiredStringParameter(args, "query", out var query, out var requiredError)) return CreateToolErrorResponse(id, requiredError!); + if (query.Length > QueryLimits.MaxQueryLength) + return CreateToolErrorResponse(id, QueryLimits.FormatQueryTooLongError()); if (IsBareVerbatimQueryToken(query)) return CreateToolErrorResponse(id, "Add a real symbol name after the command; bare verbatim prefixes like `@` are not valid queries."); @@ -3618,7 +3739,7 @@ private bool IsPathWithinClientRoots(string path) private async Task ExecuteIndexAsync(JsonNode? id, JsonNode? args, JsonNode? progressToken = null) { - if (!TryReadRequiredStringParameter(args, "path", out var path, out var requiredError)) + if (!TryReadRequiredIndexPathParameter(args, "path", out var path, out var requiredError)) return CreateToolErrorResponse(id, requiredError!); var rebuild = args?["rebuild"]?.GetValue() ?? false; diff --git a/tests/CodeIndex.Tests/McpServerTests.cs b/tests/CodeIndex.Tests/McpServerTests.cs index 2ad039817d..66c60d32f1 100644 --- a/tests/CodeIndex.Tests/McpServerTests.cs +++ b/tests/CodeIndex.Tests/McpServerTests.cs @@ -1292,6 +1292,70 @@ public void ToolCall_RequiredQuery_DistinguishesMissingFromWhitespace(string too Assert.Equal("Parameter \"query\" cannot be empty or whitespace-only", blank); } + [Theory] + [InlineData("definition")] + [InlineData("symbols")] + public void ToolCall_InvalidSince_ReturnsInvalidArgument_Issue3194(string toolName) + { + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 1, + ["method"] = "tools/call", + ["params"] = new JsonObject + { + ["name"] = toolName, + ["arguments"] = new JsonObject + { + ["query"] = "App", + ["since"] = "not-a-timestamp", + }, + }, + }; + + var response = _server.HandleMessage(request)!; + + var result = response["result"]!; + Assert.True(result["isError"]!.GetValue()); + Assert.Contains("Invalid 'since' timestamp", result["content"]![0]!["text"]!.GetValue()); + Assert.Equal(McpErrorEnvelope.CategoryInvalidArgument, result["structuredContent"]!["category"]!.GetValue()); + } + + [Fact] + public void ToolsCall_ProjectFilterResolverFailure_ReturnsInvalidArgument_Issue3160() + { + var request = JsonNode.Parse("""{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"search","arguments":{"query":"App","project":"DefinitelyMissingProject3160"}}}""")!; + + var response = _server.HandleMessage(request)!; + + var result = response["result"]!; + Assert.True(result["isError"]!.GetValue(), response.ToJsonString()); + var text = result["content"]![0]!["text"]!.GetValue(); + Assert.Contains("Project filter could not be resolved", text, StringComparison.Ordinal); + Assert.DoesNotContain("Tool 'search' failed", text, StringComparison.Ordinal); + Assert.DoesNotContain(nameof(InvalidOperationException), text, StringComparison.Ordinal); + var structured = result["structuredContent"]!; + Assert.Equal(McpErrorEnvelope.CategoryInvalidArgument, structured["category"]!.GetValue()); + Assert.Equal("project", structured["parameter"]!.GetValue()); + Assert.Contains("DefinitelyMissingProject3160", structured["diagnostic"]!.GetValue(), StringComparison.Ordinal); + } + + [Fact] + public void ToolsCall_BatchQuery_ProjectFilterResolverFailure_ReturnsSlotError_Issue3160() + { + var request = JsonNode.Parse("""{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"batch_query","arguments":{"queries":[{"tool":"search","arguments":{"query":"App","project":"DefinitelyMissingProject3160"}}]}}}""")!; + + var response = _server.HandleMessage(request)!; + + var structured = response["result"]!["structuredContent"]!; + Assert.Equal(1, structured["metadata"]!["errors"]!.GetValue()); + var slot = Assert.Single(structured["results"]!.AsArray()); + Assert.False(slot!["ok"]!.GetValue()); + Assert.Contains("Project filter could not be resolved", slot["error"]!.GetValue(), StringComparison.Ordinal); + Assert.Equal(McpErrorEnvelope.CategoryInvalidArgument, slot["category"]!.GetValue()); + Assert.Equal("project", slot["parameter"]!.GetValue()); + } + [Theory] [InlineData("outline")] [InlineData("excerpt")] @@ -1305,6 +1369,84 @@ public void ToolCall_RequiredPath_DistinguishesMissingFromWhitespace(string tool Assert.Equal("Parameter \"path\" cannot be empty or whitespace-only", blank); } + [Theory] + [InlineData("outline")] + [InlineData("excerpt")] + [InlineData("index")] + public void ToolCall_RequiredPath_RejectsNonStringType_Issue3186(string toolName) + { + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 1, + ["method"] = "tools/call", + ["params"] = new JsonObject + { + ["name"] = toolName, + ["arguments"] = BuildRequiredPathArguments(toolName, new JsonArray { "src/app.cs" }), + }, + }; + + var response = _server.HandleMessage(request)!; + + var error = response["error"]!; + Assert.Equal(-32602, error["code"]!.GetValue()); + Assert.Contains("Invalid type for argument 'path'", error["message"]!.GetValue()); + Assert.Equal("path", error["data"]!["parameter"]!.GetValue()); + } + + [Theory] + [InlineData("outline", "../outside.cs", "`..` path traversal")] + [InlineData("excerpt", "/tmp/outside.cs", "workspace-relative")] + [InlineData("index", "TOO_LONG", "must be no longer than")] + [InlineData("outline", "TOO_LONG", "must be no longer than")] + public void ToolCall_RequiredPath_RejectsInvalidPathValues_Issue3186( + string toolName, + string pathValue, + string expectedText) + { + if (pathValue == "TOO_LONG") + pathValue = new string('a', QueryCommandRunner.MaxQueryPathFilterLength + 1); + + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 1, + ["method"] = "tools/call", + ["params"] = new JsonObject + { + ["name"] = toolName, + ["arguments"] = BuildRequiredPathArguments(toolName, pathValue), + }, + }; + + var response = _server.HandleMessage(request)!; + + var result = response["result"]!; + Assert.True(result["isError"]!.GetValue(), response.ToJsonString()); + var text = result["content"]![0]!["text"]!.GetValue(); + Assert.Contains(expectedText, text, StringComparison.Ordinal); + Assert.DoesNotContain("file not found in index", text, StringComparison.OrdinalIgnoreCase); + Assert.DoesNotContain("Directory not found", text, StringComparison.Ordinal); + Assert.Equal(McpErrorEnvelope.CategoryInvalidArgument, result["structuredContent"]!["category"]!.GetValue()); + } + + [Fact] + public void ToolsList_IndexPathSchemaReflectsProjectPathContract_Issue3186() + { + var request = JsonNode.Parse("""{"jsonrpc":"2.0","id":1,"method":"tools/list"}""")!; + var response = _server.HandleMessage(request)!; + + var tools = response["result"]!["tools"]!.AsArray(); + var indexTool = tools.First(t => t!["name"]!.GetValue() == "index")!; + var pathSchema = indexTool["inputSchema"]!["properties"]!["path"]!; + + Assert.Equal("string", pathSchema["type"]!.GetValue()); + Assert.Equal(QueryCommandRunner.MaxQueryPathFilterLength, pathSchema["maxLength"]!.GetValue()); + Assert.DoesNotContain("(?!/)", pathSchema["pattern"]!.GetValue(), StringComparison.Ordinal); + Assert.Contains("absolute or relative", pathSchema["description"]!.GetValue(), StringComparison.Ordinal); + } + [Fact] public void ToolCall_FindInFilePath_DistinguishesMissingFromWhitespace() { @@ -3352,8 +3494,14 @@ public void ToolsList_CommonSchemasAdvertiseClientSideConstraints() Assert.Equal(200, searchProperties["limit"]!["maximum"]!.GetValue()); var pathStringSchema = searchProperties["path"]!["oneOf"]!.AsArray()[0]!; - Assert.Equal(4096, pathStringSchema["maxLength"]!.GetValue()); + Assert.Equal(QueryCommandRunner.MaxQueryPathFilterLength, pathStringSchema["maxLength"]!.GetValue()); Assert.NotNull(pathStringSchema["pattern"]); + var pathArraySchema = searchProperties["path"]!["oneOf"]!.AsArray()[1]!; + Assert.Equal(QueryCommandRunner.MaxQueryPathFilterCount, pathArraySchema["maxItems"]!.GetValue()); + Assert.Equal(QueryCommandRunner.MaxQueryPathFilterLength, pathArraySchema["items"]!["maxLength"]!.GetValue()); + var excludePathsSchema = searchProperties["excludePaths"]!; + Assert.Equal(QueryCommandRunner.MaxQueryPathFilterCount, excludePathsSchema["maxItems"]!.GetValue()); + Assert.Equal(QueryCommandRunner.MaxQueryPathFilterLength, excludePathsSchema["items"]!["maxLength"]!.GetValue()); var referencesTool = tools.First(t => t!["name"]!.GetValue() == "references")!; var kindEnum = referencesTool["inputSchema"]!["properties"]!["kind"]!["enum"]!.AsArray() @@ -3361,6 +3509,46 @@ public void ToolsList_CommonSchemasAdvertiseClientSideConstraints() .ToArray(); Assert.Contains("call", kindEnum); Assert.Contains("type_reference", kindEnum); + + var mapTool = tools.First(t => t!["name"]!.GetValue() == "map")!; + var sectionsSchema = mapTool["inputSchema"]!["properties"]!["sections"]!; + Assert.Equal(QueryCommandRunner.MaxQueryPathFilterCount, sectionsSchema["maxItems"]!.GetValue()); + Assert.Equal(QueryCommandRunner.MaxQueryPathFilterLength, sectionsSchema["items"]!["maxLength"]!.GetValue()); + } + + [Theory] + [InlineData("search", """{"query":"App","limit":0}""", "limit", 1, 0)] + [InlineData("definition", """{"query":"App","limit":-1}""", "limit", 1, -1)] + [InlineData("references", """{"query":"App","offset":-1}""", "offset", 0, -1)] + public void ToolsCall_InvalidLimitOrOffsetBounds_ReturnsInvalidParams_Issue3195( + string toolName, + string argumentsJson, + string parameter, + int minimum, + int actual) + { + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 1, + ["method"] = "tools/call", + ["params"] = new JsonObject + { + ["name"] = toolName, + ["arguments"] = JsonNode.Parse(argumentsJson), + }, + }; + + var response = _server.HandleMessage(request)!; + + var error = response["error"]!; + Assert.Equal(-32602, error["code"]!.GetValue()); + Assert.Contains($"Argument '{parameter}'", error["message"]!.GetValue()); + var data = error["data"]!; + Assert.Equal(McpErrorEnvelope.CategoryInvalidArgument, data["category"]!.GetValue()); + Assert.Equal(parameter, data["parameter"]!.GetValue()); + Assert.Equal(minimum, data["minimum"]!.GetValue()); + Assert.Equal(actual, data["actual"]!.GetValue()); } [Fact] @@ -4014,6 +4202,32 @@ public void ToolsCall_BareVerbatimPrefix_IsRejected(string toolName) Assert.Contains("bare verbatim prefixes like `@` are not valid queries", text); } + [Fact] + public void ToolsCall_ImpactAnalysis_RejectsOversizedQuery_Issue3184() + { + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 1, + ["method"] = "tools/call", + ["params"] = new JsonObject + { + ["name"] = "impact_analysis", + ["arguments"] = new JsonObject + { + ["query"] = new string('a', QueryLimits.MaxQueryLength + 1), + }, + }, + }; + + var response = _server.HandleMessage(request)!; + + var result = response["result"]!; + Assert.True(result["isError"]!.GetValue()); + Assert.Equal(QueryLimits.FormatQueryTooLongError(), result["content"]![0]!["text"]!.GetValue()); + Assert.Equal(McpErrorEnvelope.CategoryInvalidArgument, result["structuredContent"]!["category"]!.GetValue()); + } + [Fact] public void ToolsCall_Search_SnippetLinesControlsExcerptLength() { @@ -7855,6 +8069,70 @@ public void ToolsCall_RejectsOversizedPathArrays_Issue2028() Assert.Equal(1, structured["invalid_count"]!.GetValue()); } + [Fact] + public void ToolsCall_PathListArgumentsUseCliBounds_Issue3182() + { + Assert.Equal(QueryCommandRunner.MaxQueryPathFilterCount, McpServer.MaxMcpArrayFilterCount); + Assert.Equal(QueryCommandRunner.MaxQueryPathFilterLength, McpServer.MaxMcpArrayFilterStringLength); + + var tooManyPaths = new JsonArray(); + for (var i = 0; i < QueryCommandRunner.MaxQueryPathFilterCount + 1; i++) + tooManyPaths.Add($"src/{i}.cs"); + AssertListError( + "search", + new JsonObject + { + ["query"] = "App", + ["path"] = tooManyPaths, + }, + "path must contain at most", + expectedInvalidCount: 1); + + AssertListError( + "search", + new JsonObject + { + ["query"] = "App", + ["excludePaths"] = new JsonArray { new string('a', QueryCommandRunner.MaxQueryPathFilterLength + 1) }, + }, + $"Entries must be non-empty strings no longer than {QueryCommandRunner.MaxQueryPathFilterLength} characters.", + expectedInvalidCount: 1); + + AssertListError( + "map", + new JsonObject + { + ["sections"] = new JsonArray { 42 }, + }, + "sections contains 1 invalid entry", + expectedInvalidCount: 1); + + void AssertListError(string toolName, JsonObject arguments, string expectedText, int expectedInvalidCount) + { + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 1, + ["method"] = "tools/call", + ["params"] = new JsonObject + { + ["name"] = toolName, + ["arguments"] = arguments, + }, + }; + + var response = _server.HandleMessage(request)!; + + var result = response["result"]!; + Assert.True(result["isError"]!.GetValue(), response.ToJsonString()); + var text = result["content"]![0]!["text"]!.GetValue(); + Assert.Contains(expectedText, text); + var structured = result["structuredContent"]!; + Assert.Equal(McpErrorEnvelope.CategoryInvalidArgument, structured["category"]!.GetValue()); + Assert.Equal(expectedInvalidCount, structured["invalid_count"]!.GetValue()); + } + } + [Fact] public void ToolsCall_BatchQuery_TruncatesAggregateResponse_Issue1416() { @@ -12096,6 +12374,27 @@ public void ToolsCall_RateLimit_KeysByTool() Assert.Null(languages["error"]); } + [Fact] + public void ToolsCall_RateLimitPrecedesProjectFilterResolution_Issue3160() + { + InstallRateLimiter(_server, new RateLimiterOptions { RefillTokensPerSecond = 1.0, BurstCapacity = 1.0 }); + + var initialize = JsonNode.Parse("""{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"clientInfo":{"name":"client-a","version":"1.2.3"}}}""")!; + _server.HandleMessage(initialize); + + var first = _server.HandleMessage(JsonNode.Parse( + """{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"search","arguments":{"query":"App"}}}""")!)!; + Assert.Null(first["error"]); + + var second = _server.HandleMessage(JsonNode.Parse( + """{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"search","arguments":{"query":"App","project":"DefinitelyMissingProject3160"}}}""")!)!; + + var error = second["error"]!; + Assert.Equal(-32000, error["code"]!.GetValue()); + Assert.Contains("Rate limit exceeded", error["message"]!.GetValue()); + Assert.DoesNotContain("Project filter could not be resolved", second.ToJsonString(), StringComparison.Ordinal); + } + [Fact] public void Initialize_CapturesClientInfoAsCallerIdentity() { @@ -12862,6 +13161,20 @@ public void RegisterShutdownHandlers_AfterDispose_DoesNotInvokeHandler() Assert.False(cts.IsCancellationRequested); } + private static JsonObject BuildRequiredPathArguments(string toolName, string pathValue) + => BuildRequiredPathArguments(toolName, JsonValue.Create(pathValue)!); + + private static JsonObject BuildRequiredPathArguments(string toolName, JsonNode pathValue) + { + var arguments = new JsonObject + { + ["path"] = pathValue, + }; + if (toolName == "excerpt") + arguments["startLine"] = 1; + return arguments; + } + private string CallToolAndReadErrorMessage(string toolName, JsonObject arguments) { var request = new JsonObject diff --git a/tests/CodeIndex.Tests/McpToolContractTests.cs b/tests/CodeIndex.Tests/McpToolContractTests.cs index a60d6fa9e5..066cde44e1 100644 --- a/tests/CodeIndex.Tests/McpToolContractTests.cs +++ b/tests/CodeIndex.Tests/McpToolContractTests.cs @@ -100,6 +100,96 @@ public void ToolsList_AdvertisedInputPropertiesHaveMatchingTypeValidation_Issue3 "MCP tools/list schema and argument type validator drift detected:\n" + string.Join('\n', failures)); } + [Fact] + public void ToolsList_SearchCursorHasSharedArgumentContract_Issue3192() + { + var searchProperties = GetAdvertisedToolSchemas()["search"]; + var allowed = GetAllowedToolArguments("search"); + var (hasValidator, validatorType) = TryGetExpectedJsonType("search", "cursor"); + + Assert.True(searchProperties.ContainsKey("cursor")); + Assert.Contains("cursor", allowed); + Assert.Equal("string", ExpectedTypeFromSchema(searchProperties["cursor"])); + Assert.True(hasValidator); + Assert.Equal("string", validatorType); + } + + [Fact] + public void ToolsList_DepsArgumentsHaveSharedArgumentContract_Issue3196() + { + var depsProperties = GetAdvertisedToolSchemas()["deps"]; + var allowed = GetAllowedToolArguments("deps"); + + foreach (var argumentName in new[] { "reverse", "format", "cycles" }) + { + Assert.True(depsProperties.ContainsKey(argumentName)); + Assert.Contains(argumentName, allowed); + } + + Assert.False(depsProperties.ContainsKey("direction")); + Assert.DoesNotContain("direction", allowed); + Assert.False(depsProperties.ContainsKey("includeGenerated")); + Assert.DoesNotContain("includeGenerated", allowed); + + Assert.Equal("boolean", ExpectedTypeFromSchema(depsProperties["reverse"])); + Assert.Equal("string", ExpectedTypeFromSchema(depsProperties["format"])); + Assert.Equal("boolean", ExpectedTypeFromSchema(depsProperties["cycles"])); + Assert.Equal((true, "boolean"), TryGetExpectedJsonType("deps", "reverse")); + Assert.Equal((true, "string"), TryGetExpectedJsonType("deps", "format")); + Assert.Equal((true, "boolean"), TryGetExpectedJsonType("deps", "cycles")); + } + + [Fact] + public void ToolsList_MapSectionsAndDepthHaveSharedArgumentContract_Issue3197() + { + var mapProperties = GetAdvertisedToolSchemas()["map"]; + var allowed = GetAllowedToolArguments("map"); + + Assert.True(mapProperties.ContainsKey("sections")); + Assert.Contains("sections", allowed); + Assert.Equal("array", ExpectedTypeFromSchema(mapProperties["sections"])); + Assert.Contains("sections", SpecializedListValidatedArguments); + Assert.Equal((false, string.Empty), TryGetExpectedJsonType("map", "sections")); + + Assert.True(mapProperties.ContainsKey("depth")); + Assert.Contains("depth", allowed); + Assert.Equal("integer", ExpectedTypeFromSchema(mapProperties["depth"])); + Assert.Equal((true, "integer"), TryGetExpectedJsonType("map", "depth")); + } + + [Fact] + public void ToolsList_OutlineAndValidateDoNotExposeHiddenNoopArguments_Issue3198() + { + var advertisedSchemas = GetAdvertisedToolSchemas(); + + AssertToolArgumentsExactly(advertisedSchemas, "outline", ["path"]); + AssertToolArgumentsExactly(advertisedSchemas, "validate", ["kind", "path", "excludePaths", "excludeTests", "project", "solution"]); + + foreach (var toolName in new[] { "outline", "validate" }) + { + var advertised = advertisedSchemas[toolName].Keys.ToHashSet(StringComparer.Ordinal); + var allowed = GetAllowedToolArguments(toolName); + foreach (var noopArgument in new[] { "limit", "includeImports", "maxLineWidth", "lang" }) + { + Assert.DoesNotContain(noopArgument, advertised); + Assert.DoesNotContain(noopArgument, allowed); + } + } + + static void AssertToolArgumentsExactly( + Dictionary> advertisedSchemas, + string toolName, + string[] expectedArguments) + { + var expected = expectedArguments.ToHashSet(StringComparer.Ordinal); + var advertised = advertisedSchemas[toolName].Keys.ToHashSet(StringComparer.Ordinal); + var allowed = GetAllowedToolArguments(toolName); + + Assert.Equal(expected.Order(StringComparer.Ordinal), advertised.Order(StringComparer.Ordinal)); + Assert.Equal(expected.Order(StringComparer.Ordinal), allowed.Order(StringComparer.Ordinal)); + } + } + private static Dictionary> GetAdvertisedToolSchemas() { using var server = new McpServer("unused.db", "test", dbPathExplicit: false, McpToolFilter.AllowAll());