Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions changelog.d/unreleased/2848.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
---
category: fixed
issues:
- 2848
affected:
- src/CodeIndex/Mcp/McpToolHandlers.cs
- tests/CodeIndex.Tests/McpServerTests.cs
---

## English

- **MCP `index` now rejects unsupported arguments (#2848)** — the tool now accepts only the implemented `path`, `rebuild`, and `maxFileBytes` arguments, so scoped updates, alternate DB paths, dry runs, and optimize requests are no longer silently ignored.

## 日本語

- **MCP `index` が未対応の引数を拒否するようになりました (#2848)** — 実装済みの `path`、`rebuild`、`maxFileBytes` だけを受け付けるようにし、scoped update、別 DB パス、dry run、optimize の指定が黙って無視されないようにしました。
16 changes: 16 additions & 0 deletions changelog.d/unreleased/2849.security.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
---
category: security
issues:
- 2849
affected:
- src/CodeIndex/Mcp/McpToolHandlers.cs
- tests/CodeIndex.Tests/McpServerTests.cs
---

## English

- **MCP `batch_query` now sanitizes slot exception messages (#2849)** — batch slot failures now use the same sanitized tool error text as standalone `tools/call`, keeping raw exception details such as paths, SQL fragments, and bound values out of MCP responses.

## 日本語

- **MCP `batch_query` がスロット例外メッセージをサニタイズするようになりました (#2849)** — batch スロットの失敗でも単独の `tools/call` と同じサニタイズ済みエラー文を使い、パス、SQL 断片、バインド値などの生の例外詳細が MCP レスポンスに出ないようにしました。
20 changes: 10 additions & 10 deletions src/CodeIndex/Mcp/McpToolHandlers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -633,7 +633,7 @@ private static string DescribeJsonType(JsonNode? node)
"validate" => new HashSet<string>(StringComparer.Ordinal) { "path", "lang", "limit", "excludePaths", "excludeTests", "project", "solution" },
"unused_symbols" => new HashSet<string>(StringComparer.Ordinal) { "kind", "lang", "limit", "path", "excludePaths", "excludeTests", "project", "solution" },
"symbol_hotspots" => new HashSet<string>(StringComparer.Ordinal) { "kind", "lang", "limit", "groupBy", "path", "excludePaths", "excludeTests", "project", "solution" },
"index" => new HashSet<string>(StringComparer.Ordinal) { "path", "db", "rebuild", "parallelism", "maxFileBytes", "files", "commits", "changedBetween", "dryRun", "optimize" },
"index" => new HashSet<string>(StringComparer.Ordinal) { "path", "rebuild", "maxFileBytes" },
"backfill_fold" => new HashSet<string>(StringComparer.Ordinal) { "dry_run", "dryRun", "force" },
"suggest_improvement" => new HashSet<string>(StringComparer.Ordinal) { "category", "language", "description", "context", "toolInvocationContext", "evidencePaths", "evidence_paths" },
_ => new HashSet<string>(StringComparer.Ordinal),
Expand Down Expand Up @@ -2729,16 +2729,16 @@ void AppendRateLimitedSlot(int requestIndex, string? toolName, JsonNode? toolArg
}
catch (Exception ex)
{
// #1581: classify the exception so the slot carries the same `category`
// envelope as a standalone tools/call would. The wire message stays the raw
// ex.Message — batch_query slot errors did not pass through the #1530
// sanitizer, so keeping it is unchanged behavior; the classification is purely
// additive metadata that lets clients branch on retry-safe failures.
// #1581: 例外をカテゴリに分類して、独立した tools/call 呼び出しと同じ
// envelope を batch_query スロットでも提供する。`ex.Message` の取り扱いは
// #1530 サニタイザを通っていない既存挙動を維持し、追加メタデータのみを載せる。
// #2849: classify and sanitize slot exceptions the same way standalone
// tools/call does, so bound values, paths, and SQL/content snippets stay
// in stderr instead of the batch_query response.
DeferFrameLog(() =>
{
WriteMcpLogLine(BuildToolErrorLog(toolName, ex.Message));
Database.DbDebug.DumpToStderr(ex);
});
var classification = McpErrorEnvelope.ClassifyException(ex);
AppendSlotError(requestIndex, toolName, toolArgs, slotStopwatch, ex.Message,
AppendSlotError(requestIndex, toolName, toolArgs, slotStopwatch, BuildSanitizedToolErrorMessage(toolName, ex),
category: classification.Category,
suggestion: classification.Suggestion,
retrySafe: classification.RetrySafe);
Expand Down
77 changes: 77 additions & 0 deletions tests/CodeIndex.Tests/McpServerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6844,6 +6844,37 @@ public void ToolsCall_BatchQuery_CountsFailuresInEnvelope_Issue1537()
Assert.Contains("1 succeeded, 2 failed", text);
}

[Fact]
public void ToolsCall_BatchQuery_SanitizesSlotExceptionMessage_Issue2849()
{
const string secret = "SECRET_BATCH_SLOT_2849";
var corruptDbPath = Path.Combine(Path.GetTempPath(), $"cdidx_mcp_corrupt_{Guid.NewGuid():N}.db");
File.WriteAllText(corruptDbPath, $"not a sqlite database {secret}");
var previous = Environment.GetEnvironmentVariable(McpServer.DebugEnvironmentVariable);
try
{
Environment.SetEnvironmentVariable(McpServer.DebugEnvironmentVariable, null);
using var server = new McpServer(corruptDbPath, ConsoleUi.LoadVersion(), dbPathExplicit: true);
var request = JsonNode.Parse("""{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"batch_query","arguments":{"queries":[{"tool":"status"}]}}}""")!;

var response = server.HandleMessage(request)!;

var structured = response["result"]!["structuredContent"]!;
Assert.Equal(1, structured["failure_count"]!.GetValue<int>());
var slot = structured["results"]!.AsArray().Single()!;
var error = slot["error"]!.GetValue<string>();
Assert.Equal("Tool 'status' failed. See cdidx server stderr for details.", error);
Assert.DoesNotContain(secret, error);
Assert.DoesNotContain("file is not a database", error, StringComparison.OrdinalIgnoreCase);
Assert.Equal(McpErrorEnvelope.CategoryIndexCorrupted, slot["category"]!.GetValue<string>());
}
finally
{
Environment.SetEnvironmentVariable(McpServer.DebugEnvironmentVariable, previous);
DeleteFileRobust(corruptDbPath);
}
}

[Fact]
public void ToolsCall_BatchQuery_RejectsTypeMismatchedInnerArguments_Issue1615()
{
Expand Down Expand Up @@ -7101,6 +7132,52 @@ public void ToolsCall_Index_MissingPath_ReturnsError()
Assert.True(response["result"]!["isError"]!.GetValue<bool>());
}

[Theory]
[InlineData("db")]
[InlineData("parallelism")]
[InlineData("files")]
[InlineData("commits")]
[InlineData("changedBetween")]
[InlineData("dryRun")]
[InlineData("optimize")]
public void ToolsCall_Index_RejectsUnsupportedArguments_Issue2848(string argumentName)
{
var arguments = new JsonObject
{
["path"] = ".",
[argumentName] = argumentName switch
{
"db" => JsonValue.Create("alternate.db"),
"parallelism" => JsonValue.Create(2),
"files" => new JsonArray(JsonValue.Create("src/app.cs")),
"commits" => new JsonArray(JsonValue.Create("HEAD")),
"changedBetween" => new JsonArray(JsonValue.Create("HEAD~1"), JsonValue.Create("HEAD")),
"dryRun" => JsonValue.Create(true),
"optimize" => JsonValue.Create(true),
_ => throw new ArgumentOutOfRangeException(nameof(argumentName), argumentName, null),
},
};
var request = new JsonObject
{
["jsonrpc"] = "2.0",
["id"] = 1,
["method"] = "tools/call",
["params"] = new JsonObject
{
["name"] = "index",
["arguments"] = arguments,
},
};

var response = _server.HandleMessage(request)!;

Assert.True(response["result"]!["isError"]!.GetValue<bool>());
var text = response["result"]!["content"]![0]!["text"]!.GetValue<string>();
Assert.Contains($"Unknown argument '{argumentName}' for tool 'index'.", text);
var structured = response["result"]!["structuredContent"]!;
Assert.Equal(argumentName, structured["unknown_argument"]!.GetValue<string>());
}

[Fact]
public void ToolsCall_Index_WhenDbLockHeld_ReturnsBusyError()
{
Expand Down
Loading