diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index bdbe9ecab5..08b8ec4d2b 100644 --- a/DEVELOPER_GUIDE.md +++ b/DEVELOPER_GUIDE.md @@ -1767,11 +1767,14 @@ Piping `{"jsonrpc":"2.0","id":1,"method":"initialize","params":{}}` into caller as `stdio` / `local`). Setting `CDIDX_MCP_AUTH_TOKEN` swaps in `TokenMcpAuthenticator` for stdio, which requires every responded request to carry a matching `params.auth.token` and compares it in constant time - via `CryptographicOperations.FixedTimeEquals`. HTTP does not also use this - body-token gate: `ProgramRunner` resolves a bearer secret for the HTTP - transport from `CDIDX_MCP_HTTP_TOKEN`, falling back to `CDIDX_MCP_AUTH_TOKEN` - when the HTTP-specific variable is unset, and then relies on the - `Authorization: Bearer ...` transport check (#3156). For the JSON-RPC + via `CryptographicOperations.FixedTimeEquals`. Unset or empty configured + tokens keep the stdio gate disabled, while configured tokens must be 1-4096 + characters and cannot contain whitespace or control characters (#3505). + HTTP does not also use this body-token gate: `ProgramRunner` resolves a + bearer secret for the HTTP transport from `CDIDX_MCP_HTTP_TOKEN`, falling + back to `CDIDX_MCP_AUTH_TOKEN` when the HTTP-specific variable is unset, + and then relies on the `Authorization: Bearer ...` transport check (#3156). + For the JSON-RPC body-token gate, failures uniformly return JSON-RPC `-32001 "Unauthorized"` (per #1530 sanitization — the wire never distinguishes missing-from-wrong), and `BuildAuthFailureLog` @@ -1866,8 +1869,12 @@ return `-32600`. HTTP falls back to `CDIDX_MCP_AUTH_TOKEN` as the bearer secret; when both are set, `CDIDX_MCP_HTTP_TOKEN` wins. HTTP clients never need to also send `params.auth.token`. The CLI refuses to bind to a non-loopback host without - either token to keep the MCP catalog off the LAN by default. Configured and - supplied tokens over 4096 characters are rejected before hashing. + either token to keep the MCP catalog off the LAN by default. Unset or empty + configured bearer tokens disable the HTTP token gate where allowed by the + listen host policy, while configured tokens must be 1-4096 characters and + cannot contain whitespace or control characters (#3505). Supplied HTTP + bearer values are compared exactly after the `Bearer ` prefix: they are not + trimmed, and invalid-shape or oversized values are rejected before hashing. - Optional request-loop logging: `ProgramRunner` connects `HttpMcpTransport` to `GlobalToolLog`, so persistent logging records one `mcp_http_request` line per HTTP request when the lifecycle log is enabled. The record includes @@ -3642,7 +3649,7 @@ sequenceDiagram - `initialize` レスポンスは `protocolVersion`、`capabilities`、`serverInfo.name`、`serverInfo.version`（`ConsoleUi.LoadVersion()` — `version.json` が源）、および AI クライアントにツール選択を案内する長い `instructions` 文字列を返す。レスポンスを書き終えた後、サーバーはセッションごとに 1 回だけ互換性用の `notifications/initialized` ready signal を送るため、サーバー側の ready signal を待つクライアントも optimistic polling なしで進める（#1780）。MCP は `notifications/initialized` を client-to-server 通知としても定義しており、cdidx はその方向も no-op として受理する。非 HTTP transport では、この互換性 signal が唯一の server-origin emission です。HTTP session は `CDIDX_MCP_KEEP_ALIVE_INTERVAL_S` を設定した場合、opt-in の keep-alive notification も `/events` で受け取れる。HTTP transport では out-of-band 通知は接続済みの `/events` SSE stream にだけ配送され、POST のみのクライアントは initialize response だけを受け取り、別通知 frame は受け取らない。 - advertised capability には `tools`、`resources`、`prompts`、`logging` が含まれる。`logging` は MCP `notifications/message` を示し、`logging/setLevel` は `debug`、`info`、`notice`、`warning`、`error`、`critical`、`alert`、`emergency` を受け付ける。 - `protocolVersion` は**ハードコードではなく交渉**で決まる（#1554）。サーバーは `McpServer.SupportedProtocolVersions`（新しい順: `2025-03-26`, `2024-11-05`）を保持し、`initialize` パラメータからクライアント要求バージョンを読み取って、対応集合にあればそれを返し（合意）、未指定／非文字列なら既定の最新バージョンに fallback し、対応外なら `error.data` に `requestedVersion` と `supportedVersions` を入れた JSON-RPC `-32602` で拒否する。これにより将来 MCP 仕様が改訂されても、wire format が黙ってずれるのではなく actionable な handshake 失敗として表面化する。配列を新バージョンで更新する際は `ProtocolVersion` を先頭エントリと揃えて意図的に bump する。 -- **認証ミドルウェア**（#1559）。`McpServer` はパース済み JSON-RPC リクエストごとに、メソッド抽出 *後*・dispatch *前* で `IMcpAuthenticator` を呼ぶ。既定の `LocalStdioAuthenticator` は permissive で（従来の stdio 動作を維持し、呼び出し元を `stdio` / `local` でタグ付けする）、stdio では `CDIDX_MCP_AUTH_TOKEN` を設定すると `TokenMcpAuthenticator` に切り替わる。`TokenMcpAuthenticator` は応答が必要な全リクエストに対し、`params.auth.token` が一致することを要求し、比較は `CryptographicOperations.FixedTimeEquals` による定数時間比較で行う。HTTP はこの body token ゲートを重ねず、`ProgramRunner` が `CDIDX_MCP_HTTP_TOKEN` を優先し、未設定なら `CDIDX_MCP_AUTH_TOKEN` を fallback として bearer secret に解決して、`Authorization: Bearer ...` の transport check に一本化する（#3156）。JSON-RPC body token ゲートの失敗は統一された JSON-RPC `-32001 "Unauthorized"` を返し（#1530 の sanitization 方針に従い、ワイヤでは未提示と不一致を区別しない）、`BuildAuthFailureLog` が詳細を stderr に書き出す。通知（`notifications/initialized`、`notifications/cancelled`）は応答もエラーコードも持たないため、ゲート *より前* で short-circuit する。このミドルウェアが将来 transport の差し替え seam になる — ネットワーク listener は別の `IMcpAuthenticator` を提供しつつ、`McpCallerIdentity`（`Source` + `Subject`）の形を保ち、監査ログ（#1562）から再利用できる。 +- **認証ミドルウェア**（#1559）。`McpServer` はパース済み JSON-RPC リクエストごとに、メソッド抽出 *後*・dispatch *前* で `IMcpAuthenticator` を呼ぶ。既定の `LocalStdioAuthenticator` は permissive で（従来の stdio 動作を維持し、呼び出し元を `stdio` / `local` でタグ付けする）、stdio では `CDIDX_MCP_AUTH_TOKEN` を設定すると `TokenMcpAuthenticator` に切り替わる。未設定または空文字の token だけが permissive で、空白のみ・空白文字入り・制御文字入り・4096 文字超の token は設定値として拒否する。`TokenMcpAuthenticator` は応答が必要な全リクエストに対し、`params.auth.token` が一致することを要求し、比較は `CryptographicOperations.FixedTimeEquals` による定数時間比較で行う。HTTP はこの body token ゲートを重ねず、`ProgramRunner` が `CDIDX_MCP_HTTP_TOKEN` を優先し、未設定なら `CDIDX_MCP_AUTH_TOKEN` を fallback として bearer secret に解決して、`Authorization: Bearer ...` の transport check に一本化する（#3156）。HTTP bearer 値は `Bearer ` の後ろを trim せず完全一致で扱い、空白文字・制御文字・4096 文字超は hash 前に拒否する。JSON-RPC body token ゲートの失敗は統一された JSON-RPC `-32001 "Unauthorized"` を返し（#1530 の sanitization 方針に従い、ワイヤでは未提示と不一致を区別しない）、`BuildAuthFailureLog` が詳細を stderr に書き出す。通知（`notifications/initialized`、`notifications/cancelled`）は応答もエラーコードも持たないため、ゲート *より前* で short-circuit する。このミドルウェアが将来 transport の差し替え seam になる — ネットワーク listener は別の `IMcpAuthenticator` を提供しつつ、`McpCallerIdentity`（`Source` + `Subject`）の形を保ち、監査ログ（#1562）から再利用できる。 MCP は独立したシリアライズ戦略（オブジェクトを JSON などの転送形式に変換する方式のこと。CLI の `--json` 側は .NET 標準の `JsonSerializer` に任せる方式、MCP 側は `JsonObject` を手で組み立てる方式と、別の手段を採っている）を採るため、「そもそもバイナリは走るのか?」を確かめる最も頑健なスモークテスト（デプロイや起動直後に行う、基本動作だけを短時間で確認する簡易テストのこと。詳細な正しさではなく「煙が出ていないか＝致命的に壊れていないか」を見るためこの名で呼ばれる）となる — .NET ホスト、`Program.Main`、CLI ルーティング、`ConsoleUi.LoadVersion()` に負荷をかけるが、SQLite には触れない（`search` など MCP の*ツール呼び出し*は SQLite に触れるが、`initialize` 単独では触れない）。 @@ -3662,7 +3669,7 @@ MCP は独立したシリアライズ戦略（オブジェクトを JSON など - HTTP POST 1 件 = JSON-RPC フレーム 1 件で、対応する応答は HTTP レスポンスのボディ（`200 OK` / `application/json; charset=utf-8`）に乗る。通知は `204 No Content`。`GET /events` は将来のサーバー→クライアント frame 用に独立した `text/event-stream` subscription を開く。サーバーは `CDIDX_MCP_KEEP_ALIVE_INTERVAL_S` で keep-alive notification が opt-in された場合を除き、自発的な frame を送信しない。長寿命の event stream は通常の POST リクエストを塞がない。`/` への POST 以外は `405 Method Not Allowed`。空 / 空白のみのボディは stdio の空行と同じ扱いで `204 No Content` を返し、ループは殺さない — クライアントの誤動作で junk フレームに引っかからないため。リクエスト本文は `CDIDX_MCP_HTTP_MAX_REQUEST_BYTES`（既定: 1,000,000 bytes、最大: 16,777,216 bytes）で制限し、超過時は全量を buffer する前に `413 Payload Too Large` を返す。保留中 request queue は `CDIDX_MCP_HTTP_MAX_QUEUE_DEPTH`（既定: 64、最大: 1,024）、受理済み context handler task は `CDIDX_MCP_HTTP_MAX_CONCURRENT_HANDLERS`（既定: 64、最大: 1,024）、同時 `/events` stream は `CDIDX_MCP_HTTP_MAX_EVENT_STREAMS`（既定: 16、最大: 1,024）で制限し、満杯時は無制限に work を保持せず `Retry-After: 1` 付きの `429 Too Many Requests` を返す。正でない値や数値でない環境変数値は既定にフォールバックし、最大値を超える値は listener 起動前に拒否する。 - SSE stream lifetime は active stream registry と上限付き active-stream counter だけで表現する。idle stream には最小限の SSE comment heartbeat を送り、切断済み client を検出して stream slot を解放する。その registry entry が削除された後に完了済み stream task を保持しない。 - `ResolveListenSpec("host:port")` は prefix を事前に解決するため、CLI が stderr に `Listening on http://...` を出せる。ポート `0` は一時 `TcpListener` を probe して空きポートを取得する。probe から `HttpListener.Start()` までの TOCTOU window は、本トランスポートが local-only / single-tenant 想定であるため許容する。ワイルドカードホスト `+` / `*` はパース時点で拒否する。 -- 任意の共有秘密による認証: `CDIDX_MCP_HTTP_TOKEN` が設定されていれば、listener はすべてのリクエストに `Authorization: Bearer ` を要求し、定数時間で比較する。`CDIDX_MCP_HTTP_TOKEN` が未設定なら HTTP は `CDIDX_MCP_AUTH_TOKEN` を bearer secret として fallback し、両方が設定されている場合は `CDIDX_MCP_HTTP_TOKEN` を優先する。HTTP クライアントが `params.auth.token` も送る必要はない。どちらのトークンも未指定で非 loopback ホストへ bind しようとした場合、CLI は MCP カタログを LAN に漏らさないよう既定で拒否する。設定 token と受信 token は 4096 文字を超える場合、hash 前に拒否する。 +- 任意の共有秘密による認証: `CDIDX_MCP_HTTP_TOKEN` が設定されていれば、listener はすべてのリクエストに `Authorization: Bearer ` を要求し、定数時間で比較する。`CDIDX_MCP_HTTP_TOKEN` が未設定なら HTTP は `CDIDX_MCP_AUTH_TOKEN` を bearer secret として fallback し、両方が設定されている場合は `CDIDX_MCP_HTTP_TOKEN` を優先する。HTTP クライアントが `params.auth.token` も送る必要はない。どちらのトークンも未指定または空文字で非 loopback ホストへ bind しようとした場合、CLI は MCP カタログを LAN に漏らさないよう既定で拒否する。設定 token は 1-4096 文字で、空白文字・制御文字を含んではならない。受信 bearer token も trim せず完全一致で扱い、空白文字・制御文字・4096 文字超は hash 前に拒否する。 - 任意のリクエストループログ: `ProgramRunner` は `HttpMcpTransport` を `GlobalToolLog` に接続するため、lifecycle log が有効な場合は HTTP リクエストごとに `mcp_http_request` 行を 1 件記録する。記録内容は method、path、status、duration、auth outcome、remote peer、correlation id、利用可能な JSON-RPC request id で、caller-controlled な method、path、remote peer、request id は 256 文字を上限に `...` marker 付きで切り詰める。リクエスト/レスポンス本文は含めない。 - キャンセルは `_listener.Stop()` に接続するため、シャットダウン時に `GetContextAsync()` が unblock する。`HttpListenerException` / `ObjectDisposedException` は EOS と同じ扱いで MCP ループを stdin クローズと同じ経路で終了させる。 diff --git a/USER_GUIDE.md b/USER_GUIDE.md index 30ed7283e6..f4e9bc3a90 100644 --- a/USER_GUIDE.md +++ b/USER_GUIDE.md @@ -2220,7 +2220,7 @@ Security defaults: - The listener binds to a loopback address (`127.0.0.1`) by default, and the wildcard hosts `+` / `*` are rejected outright. - Binding to a non-loopback host (e.g. `0.0.0.0:9000`) is refused unless you set `CDIDX_MCP_HTTP_TOKEN` or `CDIDX_MCP_AUTH_TOKEN` to a shared secret. `CDIDX_MCP_HTTP_TOKEN` wins when both are set. When an HTTP bearer secret is configured, every request must carry `Authorization: Bearer ` or the listener returns `401 Unauthorized` with `WWW-Authenticate: Bearer realm="cdidx-mcp"`; HTTP clients do not also need `params.auth.token`. -- The configured token's SHA-256 digest is precomputed at start-up; per-request authentication only hashes the supplied input and compares against the stored digest in constant time, so neither the configured token's length nor its bytes leak through timing. Configured and supplied tokens longer than 4096 characters are rejected before hashing. +- The configured token's SHA-256 digest is precomputed at start-up; per-request authentication only hashes the supplied input and compares against the stored digest in constant time, so neither the configured token's length nor its bytes leak through timing. Leaving the token variable unset, or setting it to the empty string, disables the token gate. Any configured token must be 1-4096 characters and must not contain whitespace or control characters. Supplied HTTP bearer values use the exact bytes after `Bearer ` and are not trimmed before comparison; oversized, whitespace-containing, or control-character-bearing values are rejected before hashing. The stdio transport stays byte-for-byte unchanged, so existing client configs keep working without modification. @@ -2228,9 +2228,9 @@ The stdio transport stays byte-for-byte unchanged, so existing client configs ke `CDIDX_MCP_HTTP_TOKEN` above guards the HTTP transport at the `Authorization: Bearer ...` header. If it is unset, HTTP uses `CDIDX_MCP_AUTH_TOKEN` as the bearer secret instead. For stdio, `CDIDX_MCP_AUTH_TOKEN` enables the JSON-RPC-level auth gate (#1559). -The default `cdidx mcp` server is **permissive** — the OS-enforced stdio process boundary already gates access, and every existing client setup above (Claude Code, Cursor, Windsurf, Copilot, Codex) keeps working unchanged. When `CDIDX_MCP_AUTH_TOKEN` is unset (or whitespace-only), the server accepts every request and tags it with the shared `stdio` / `local` caller identity. +The default `cdidx mcp` server is **permissive** — the OS-enforced stdio process boundary already gates access, and every existing client setup above (Claude Code, Cursor, Windsurf, Copilot, Codex) keeps working unchanged. When `CDIDX_MCP_AUTH_TOKEN` is unset or set to the empty string, the server accepts every request and tags it with the shared `stdio` / `local` caller identity. A whitespace-only value is invalid rather than permissive. -If you expose stdio `cdidx mcp` over a less-trusted channel (a forwarded socket, a sandbox bridge, a shared CI runner), set `CDIDX_MCP_AUTH_TOKEN` to a non-whitespace secret. The stdio server then requires every responded JSON-RPC request (`initialize`, `tools/list`, `tools/call`, `ping`) to include the same token at `params.auth.token`. HTTP uses the same variable only as a bearer-secret fallback when `CDIDX_MCP_HTTP_TOKEN` is unset, so HTTP clients send `Authorization: Bearer ` instead of duplicating the token in the JSON-RPC body. The expected token is stored as a SHA-256 digest and the presented token is hashed to the same length before `CryptographicOperations.FixedTimeEquals`, so missing / wrong-length / wrong-value guesses share one constant-time path and neither token length nor bytes leak through timing. Mismatches return a uniform JSON-RPC `-32001 "Unauthorized"` — the wire body never distinguishes "missing token" from "wrong token", so the response cannot be used as a token-existence oracle (#1530). The detailed failure reason is written to `cdidx mcp` stderr for local diagnostics, with `method` sanitized to strip control characters so a malicious request body cannot forge log lines. Notifications (`notifications/initialized`, `notifications/cancelled`) skip the gate because they have no `id` and cannot signal an error code. +If you expose stdio `cdidx mcp` over a less-trusted channel (a forwarded socket, a sandbox bridge, a shared CI runner), set `CDIDX_MCP_AUTH_TOKEN` to a 1-4096 character secret with no whitespace or control characters. The stdio server then requires every responded JSON-RPC request (`initialize`, `tools/list`, `tools/call`, `ping`) to include the same token at `params.auth.token`. HTTP uses the same variable only as a bearer-secret fallback when `CDIDX_MCP_HTTP_TOKEN` is unset, so HTTP clients send `Authorization: Bearer ` instead of duplicating the token in the JSON-RPC body. HTTP bearer values are compared exactly and are not trimmed before hashing. The expected token is stored as a SHA-256 digest and the presented token is hashed to the same length before `CryptographicOperations.FixedTimeEquals`, so missing / wrong-length / wrong-value guesses share one constant-time path and neither token length nor bytes leak through timing. Mismatches return a uniform JSON-RPC `-32001 "Unauthorized"` — the wire body never distinguishes "missing token" from "wrong token", so the response cannot be used as a token-existence oracle (#1530). The detailed failure reason is written to `cdidx mcp` stderr for local diagnostics, with `method` sanitized to strip control characters so a malicious request body cannot forge log lines. Notifications (`notifications/initialized`, `notifications/cancelled`) skip the gate because they have no `id` and cannot signal an error code. This remains useful for custom stdio MCP clients you control. Stdio clients that do not inject `params.auth.token` will be rejected once the variable is set, so leave it unset unless you actively want to enforce body-token authentication; HTTP clients should prefer the bearer-header contract above. diff --git a/changelog.d/unreleased/3370.security.md b/changelog.d/unreleased/3370.security.md new file mode 100644 index 0000000000..24b69bf244 --- /dev/null +++ b/changelog.d/unreleased/3370.security.md @@ -0,0 +1,17 @@ +--- +category: security +issues: + - 3370 +affected: + - src/CodeIndex/Mcp/McpServer.cs + - src/CodeIndex/Mcp/McpToolHandlers.cs + - tests/CodeIndex.Tests/McpServerTests.cs +--- + +## English + +- **MCP tool errors no longer echo raw exception messages (#3370)** — tool error responses and MCP tool error logs now use sanitized exception details so parser, SQLite, and filesystem exception text cannot leak matched content or bound values. + +## 日本語 + +- **MCP tool エラーが raw exception message を返さないようになりました (#3370)** — tool error response と MCP tool error log はサニタイズ済みの例外詳細を使うようになり、parser / SQLite / filesystem の例外文から一致内容や bind 値が漏れないようになりました。 diff --git a/changelog.d/unreleased/3425.security.md b/changelog.d/unreleased/3425.security.md new file mode 100644 index 0000000000..5ed24d33bc --- /dev/null +++ b/changelog.d/unreleased/3425.security.md @@ -0,0 +1,20 @@ +--- +category: security +issues: + - 3425 +affected: + - src/CodeIndex/SafeDiagnosticFormatter.cs + - src/CodeIndex/Cli/QueryCommandRunner.cs + - src/CodeIndex/Indexer/Symbols/SymbolExtractionWorker.cs + - src/CodeIndex/Indexer/Hooks/PostExtractionHookCallbackWorker.cs + - tests/CodeIndex.Tests/QueryCommandRunnerTests.cs + - tests/CodeIndex.Tests/IndexCommandRunnerTests.cs +--- + +## English + +- **Worker and batch diagnostics now avoid raw exception text (#3425)** — symbol workers, hook callback workers, and `cdidx batch` JSON parse failures now report stable sanitized categories such as `worker_protocol_error` and `invalid_batch_json` instead of echoing exception messages. + +## 日本語 + +- **worker と batch 診断が raw exception text を避けるようになりました (#3425)** — symbol worker、hook callback worker、`cdidx batch` の JSON parse failure は、例外メッセージをそのまま返さず `worker_protocol_error` や `invalid_batch_json` などの安定したサニタイズ済みカテゴリを報告します。 diff --git a/changelog.d/unreleased/3469.security.md b/changelog.d/unreleased/3469.security.md new file mode 100644 index 0000000000..106af363ca --- /dev/null +++ b/changelog.d/unreleased/3469.security.md @@ -0,0 +1,16 @@ +--- +category: security +issues: + - 3469 +affected: + - src/CodeIndex/Mcp/HttpMcpTransport.cs + - tests/CodeIndex.Tests/HttpMcpTransportTests.cs +--- + +## English + +- **HTTP MCP auth logs now coarsen failures by default (#3469)** — request log auth outcomes report failed bearer authentication as `unauthorized` unless unsafe debug diagnostics are explicitly enabled. + +## 日本語 + +- **HTTP MCP auth log は既定で認証失敗を丸めるようになりました (#3469)** — request log の auth outcome は、unsafe debug 診断が明示的に有効な場合を除き、bearer 認証失敗を `unauthorized` として記録します。 diff --git a/changelog.d/unreleased/3505.security.md b/changelog.d/unreleased/3505.security.md new file mode 100644 index 0000000000..3d290c3de0 --- /dev/null +++ b/changelog.d/unreleased/3505.security.md @@ -0,0 +1,20 @@ +--- +category: security +issues: + - 3505 +affected: + - src/CodeIndex/Cli/ProgramRunner.cs + - src/CodeIndex/Mcp/McpAuthentication.cs + - src/CodeIndex/Mcp/HttpMcpTransport.cs + - tests/CodeIndex.Tests/ProgramRunnerTests.cs + - tests/CodeIndex.Tests/McpServerTests.cs + - tests/CodeIndex.Tests/HttpMcpTransportTests.cs +--- + +## English + +- **MCP auth tokens now reject ambiguous whitespace (#3505)** — configured stdio and HTTP bearer tokens must be non-empty and free of whitespace or control characters, and HTTP bearer headers no longer trim token candidates before comparison. + +## 日本語 + +- **MCP auth token が曖昧な空白を拒否するようになりました (#3505)** — stdio と HTTP bearer の設定 token は空でなく、空白や制御文字を含まない必要があります。HTTP bearer header も比較前に token candidate を trim しなくなりました。 diff --git a/changelog.d/unreleased/3506.security.md b/changelog.d/unreleased/3506.security.md new file mode 100644 index 0000000000..35c651a373 --- /dev/null +++ b/changelog.d/unreleased/3506.security.md @@ -0,0 +1,21 @@ +--- +category: security +issues: + - 3506 +affected: + - src/CodeIndex/BoundedLineReader.cs + - src/CodeIndex/Mcp/StdioMcpTransport.cs + - src/CodeIndex/Mcp/McpServer.cs + - src/CodeIndex/Indexer/Symbols/SymbolExtractionWorker.cs + - src/CodeIndex/Indexer/Hooks/PostExtractionHookCallbackWorker.cs + - tests/CodeIndex.Tests/McpServerTests.cs + - tests/CodeIndex.Tests/IndexCommandRunnerTests.cs +--- + +## English + +- **MCP stdio and worker protocols now cap lines while reading (#3506)** — line-delimited MCP and worker frames are rejected at the protocol boundary before unbounded `ReadLine` allocation can occur. + +## 日本語 + +- **MCP stdio と worker protocol が読み取り中に line cap を適用するようになりました (#3506)** — line-delimited な MCP / worker frame は、無制限の `ReadLine` 確保が起きる前に protocol 境界で拒否されます。 diff --git a/src/CodeIndex/BoundedLineReader.cs b/src/CodeIndex/BoundedLineReader.cs new file mode 100644 index 0000000000..0e0471f0d9 --- /dev/null +++ b/src/CodeIndex/BoundedLineReader.cs @@ -0,0 +1,187 @@ +using System.Text; + +namespace CodeIndex; + +internal sealed class BoundedLineLengthException : IOException +{ + internal BoundedLineLengthException(int charactersRead, int utf8BytesRead, int maxCharacters, int maxUtf8Bytes) + : base($"Line exceeds the {maxCharacters} character or {maxUtf8Bytes} byte cap.") + { + CharactersRead = charactersRead; + Utf8BytesRead = utf8BytesRead; + MaxCharacters = maxCharacters; + MaxUtf8Bytes = maxUtf8Bytes; + } + + internal int CharactersRead { get; } + + internal int Utf8BytesRead { get; } + + internal int MaxCharacters { get; } + + internal int MaxUtf8Bytes { get; } +} + +internal static class WorkerProtocolLineLimits +{ + // Worker requests carry source content as JSON. Keep this above the default 4 MiB source + // file cap after JSON escaping while still bounding line-protocol memory growth. + internal const int MaxLineCharacters = 32 * 1024 * 1024; + internal const int MaxLineUtf8Bytes = 32 * 1024 * 1024; + private const long JsonEscapedCharacterBytes = 6; + private const long ProtocolEnvelopeBytes = 1024 * 1024; + + internal static int ResolveForSourceFileBytes(long? maxFileSizeBytes) + { + if (maxFileSizeBytes is not > 0) + return MaxLineUtf8Bytes; + + var required = checked(maxFileSizeBytes.Value * JsonEscapedCharacterBytes + ProtocolEnvelopeBytes); + if (required <= MaxLineUtf8Bytes) + return MaxLineUtf8Bytes; + if (required >= int.MaxValue) + return int.MaxValue; + + return (int)required; + } +} + +internal static class BoundedLineReader +{ + internal static string? ReadLine(TextReader reader, int maxCharacters, int maxUtf8Bytes) + { + ArgumentNullException.ThrowIfNull(reader); + var state = new LineState(maxCharacters, maxUtf8Bytes); + + while (true) + { + var value = reader.Read(); + if (value < 0) + return state.HasAnyInput ? state.CompleteLine() : null; + + if (state.Process((char)value, out var line)) + return line; + } + } + + internal static async Task ReadLineAsync( + TextReader reader, + int maxCharacters, + int maxUtf8Bytes, + CancellationToken cancellationToken) + { + ArgumentNullException.ThrowIfNull(reader); + var state = new LineState(maxCharacters, maxUtf8Bytes); + var buffer = new char[1]; + + while (true) + { + var read = await reader.ReadAsync(buffer.AsMemory(0, 1), cancellationToken).ConfigureAwait(false); + if (read == 0) + return state.HasAnyInput ? state.CompleteLine() : null; + + if (state.Process(buffer[0], out var line)) + return line; + } + } + + private sealed class LineState + { + private readonly int _maxCharacters; + private readonly int _maxUtf8Bytes; + private readonly StringBuilder _builder = new(); + private readonly Encoder _utf8Encoder = Encoding.UTF8.GetEncoder(); + private int _charactersRead; + private int _utf8BytesRead; + private bool _pendingCarriageReturn; + + internal LineState(int maxCharacters, int maxUtf8Bytes) + { + if (maxCharacters <= 0) + throw new ArgumentOutOfRangeException(nameof(maxCharacters), maxCharacters, "Maximum line characters must be positive."); + if (maxUtf8Bytes <= 0) + throw new ArgumentOutOfRangeException(nameof(maxUtf8Bytes), maxUtf8Bytes, "Maximum line UTF-8 bytes must be positive."); + + _maxCharacters = maxCharacters; + _maxUtf8Bytes = maxUtf8Bytes; + } + + internal bool HasAnyInput { get; private set; } + + internal bool Process(char ch, out string? line) + { + HasAnyInput = true; + line = null; + + if (_pendingCarriageReturn) + { + if (ch == '\n') + { + _pendingCarriageReturn = false; + line = CompleteLine(); + return true; + } + + Append('\r'); + _pendingCarriageReturn = false; + } + + if (ch == '\r') + { + _pendingCarriageReturn = true; + return false; + } + + if (ch == '\n') + { + line = CompleteLine(); + return true; + } + + Append(ch); + return false; + } + + internal string CompleteLine() + { + _pendingCarriageReturn = false; + FlushEncoder(); + return _builder.ToString(); + } + + private void Append(char ch) + { + _builder.Append(ch); + _charactersRead++; + _utf8BytesRead += CountUtf8Bytes(ch, flush: false); + ThrowIfExceeded(); + } + + private void FlushEncoder() + { + _utf8BytesRead += CountUtf8Bytes(default, flush: true); + ThrowIfExceeded(); + } + + private int CountUtf8Bytes(char ch, bool flush) + { + Span bytes = stackalloc byte[8]; + if (flush) + { + _utf8Encoder.Convert(ReadOnlySpan.Empty, bytes, flush: true, out _, out var bytesUsed, out _); + return bytesUsed; + } + + Span chars = stackalloc char[1]; + chars[0] = ch; + _utf8Encoder.Convert(chars, bytes, flush: false, out _, out var used, out _); + return used; + } + + private void ThrowIfExceeded() + { + if (_charactersRead > _maxCharacters || _utf8BytesRead > _maxUtf8Bytes) + throw new BoundedLineLengthException(_charactersRead, _utf8BytesRead, _maxCharacters, _maxUtf8Bytes); + } + } +} diff --git a/src/CodeIndex/Cli/IndexCommandRunner.FullScan.cs b/src/CodeIndex/Cli/IndexCommandRunner.FullScan.cs index 57c3cd1052..b386875eb2 100644 --- a/src/CodeIndex/Cli/IndexCommandRunner.FullScan.cs +++ b/src/CodeIndex/Cli/IndexCommandRunner.FullScan.cs @@ -733,7 +733,7 @@ void ThrowIfFullScanCancelled(int filesProcessed, int? filesTotal) var activeJsonExtractionPhases = new ConcurrentDictionary(); CancellationTokenSource? jsonHeartbeatCts = null; Task? jsonHeartbeatTask = null; - using var postExtractionHooks = PostExtractionHookRunner.DiscoverDefault(); + using var postExtractionHooks = PostExtractionHookRunner.DiscoverDefault(options.MaxFileSizeBytes); var extractionParallelism = Math.Max(1, options.Parallelism); var hasPostExtractionHooks = postExtractionHooks.Hooks.Count > 0; var parallelizeExtraction = (options.Rebuild || writer.GetCounts().files == 0 || headChangeDetected) @@ -914,13 +914,13 @@ void StopJsonHeartbeat() using var extractionResults = new BlockingCollection(Math.Max(1, extractionParallelism * 4)); using var extractionStallCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); - using var mainSymbolExtractionWorker = new SymbolExtractionWorkerClient(); + using var mainSymbolExtractionWorker = new SymbolExtractionWorkerClient(options.MaxFileSizeBytes); var extractionCancellationToken = extractionStallCts.Token; var nextFileIndex = -1; var workers = Enumerable.Range(0, extractionParallelism) .Select(workerIndex => Task.Factory.StartNew(() => { - using var workerSymbolExtractionWorker = new SymbolExtractionWorkerClient(); + using var workerSymbolExtractionWorker = new SymbolExtractionWorkerClient(options.MaxFileSizeBytes); while (true) { extractionCancellationToken.ThrowIfCancellationRequested(); diff --git a/src/CodeIndex/Cli/IndexCommandRunner.Update.cs b/src/CodeIndex/Cli/IndexCommandRunner.Update.cs index b5bfa3bb3d..b8ed94fd1b 100644 --- a/src/CodeIndex/Cli/IndexCommandRunner.Update.cs +++ b/src/CodeIndex/Cli/IndexCommandRunner.Update.cs @@ -129,7 +129,7 @@ private static int RunUpdateMode( var ftsMutated = false; var purgedRefs = 0; var supportedGraphLanguages = ReferenceExtractor.GetSupportedLanguages(); - using var postExtractionHooks = PostExtractionHookRunner.DiscoverDefault(); + using var postExtractionHooks = PostExtractionHookRunner.DiscoverDefault(options.MaxFileSizeBytes); var currentFoldVersion = NameFold.Version.ToString(System.Globalization.CultureInfo.InvariantCulture); var currentFoldFingerprint = NameFold.Fingerprint(); var currentCSharpSymbolNameContractVersion = DbContext.CSharpSymbolNameContractVersion.ToString(System.Globalization.CultureInfo.InvariantCulture); @@ -318,7 +318,7 @@ void ThrowIfUpdateCancelled() () => currentUpdatePath == null ? $"{updated + removed + skipped:N0}/{targetPaths.Count:N0} files processed" : $"{updated + removed + skipped:N0}/{targetPaths.Count:N0} files processed, current {currentUpdatePath}"); - using var symbolExtractionWorker = new SymbolExtractionWorkerClient(); + using var symbolExtractionWorker = new SymbolExtractionWorkerClient(options.MaxFileSizeBytes); try { foreach (var relPath in targetPaths) diff --git a/src/CodeIndex/Cli/ProgramRunner.cs b/src/CodeIndex/Cli/ProgramRunner.cs index 6195917035..3560dbd907 100644 --- a/src/CodeIndex/Cli/ProgramRunner.cs +++ b/src/CodeIndex/Cli/ProgramRunner.cs @@ -2408,7 +2408,18 @@ private static int RunMcp(string[] cmdArgs, string appVersion) // リクエストに header token と body token の両方を要求しない。ツール有効化ゲート // (#1561) は McpServer のコンストラクタ内部で `McpToolFilter.FromEnvironment()` // から自動取得される。 - var authenticator = CreateMcpAuthenticatorForTransport(runOptions.Transport); + IMcpAuthenticator authenticator; + try + { + authenticator = CreateMcpAuthenticatorForTransport(runOptions.Transport); + } + catch (FormatException ex) + { + Console.Error.WriteLine($"Error: {ex.Message}"); + PrintMcpUsage(); + return CommandExitCodes.UsageError; + } + using var server = new McpServer(runOptions.QueryOptions.DbPath, appVersion, runOptions.QueryOptions.DbPathExplicit, authenticator, auditLog); return RunMcpServer(server, runOptions.Transport, runOptions.ListenSpec); } @@ -2581,15 +2592,23 @@ internal static IMcpAuthenticator CreateMcpAuthenticatorForTransport(string tran internal static string? ResolveMcpHttpBearerTokenFromEnvironment() { - var httpToken = NormalizeMcpToken(Environment.GetEnvironmentVariable(McpHttpTokenEnvVar)); + var httpToken = NormalizeMcpToken(Environment.GetEnvironmentVariable(McpHttpTokenEnvVar), McpHttpTokenEnvVar); if (httpToken is not null) return httpToken; - return NormalizeMcpToken(Environment.GetEnvironmentVariable(McpAuthenticatorFactory.AuthTokenEnvVar)); + return NormalizeMcpToken(Environment.GetEnvironmentVariable(McpAuthenticatorFactory.AuthTokenEnvVar), McpAuthenticatorFactory.AuthTokenEnvVar); } - private static string? NormalizeMcpToken(string? token) - => string.IsNullOrWhiteSpace(token) ? null : token; + private static string? NormalizeMcpToken(string? token, string source) + { + if (string.IsNullOrEmpty(token)) + return null; + + if (!McpAuthenticationLimits.IsTokenShapeValid(token)) + throw new FormatException(McpAuthenticationLimits.FormatTokenShapeError(source)); + + return token; + } private static int RunMcpHttp(McpServer server, string listenSpec) { @@ -2618,7 +2637,17 @@ private static int RunMcpHttp(McpServer server, string listenSpec) // HTTP は保護され、クライアントに `Authorization` と `params.auth.token` の両方を // 要求しない (#3156)。どちらの token も未設定なら、loopback bind は stdio と同等の脅威 // モデルとみなしてトークン要件を緩める。 - var bearerToken = ResolveMcpHttpBearerTokenFromEnvironment(); + string? bearerToken; + try + { + bearerToken = ResolveMcpHttpBearerTokenFromEnvironment(); + } + catch (FormatException ex) + { + Console.Error.WriteLine($"Error: {ex.Message}"); + PrintMcpUsage(); + return CommandExitCodes.UsageError; + } if (!resolved.IsLoopback && bearerToken is null) { diff --git a/src/CodeIndex/Cli/QueryCommandRunner.cs b/src/CodeIndex/Cli/QueryCommandRunner.cs index 5bf0eeecdd..7dfd12aa79 100644 --- a/src/CodeIndex/Cli/QueryCommandRunner.cs +++ b/src/CodeIndex/Cli/QueryCommandRunner.cs @@ -456,9 +456,9 @@ private static bool TryParseBatchLine(string line, int lineNumber, out string co subArgs = values.Skip(1).ToArray(); return true; } - catch (JsonException ex) + catch (JsonException) { - Console.Error.WriteLine($"Error: batch line {lineNumber} is not valid JSON: {ex.Message}"); + Console.Error.WriteLine($"Error [{CommandErrorCodes.UsageError}]: batch line {lineNumber} {SafeDiagnosticFormatter.FormatCategoryType("invalid_batch_json", nameof(JsonException))}."); return false; } } diff --git a/src/CodeIndex/Indexer/Hooks/PostExtractionHookCallbackWorker.cs b/src/CodeIndex/Indexer/Hooks/PostExtractionHookCallbackWorker.cs index c91f381947..780958de24 100644 --- a/src/CodeIndex/Indexer/Hooks/PostExtractionHookCallbackWorker.cs +++ b/src/CodeIndex/Indexer/Hooks/PostExtractionHookCallbackWorker.cs @@ -1,4 +1,5 @@ using System.Diagnostics; +using System.Globalization; using System.Reflection; using System.Runtime.Loader; using System.Runtime.Versioning; @@ -27,14 +28,16 @@ internal sealed record PostExtractionHookCallbackResult( internal sealed class PostExtractionHookCallbackWorkerClient : IDisposable { private readonly PostExtractionHookInfo hook; + private readonly int maxProtocolLineBytes; private readonly object gate = new(); private Process? process; private StringBuilder stderr = new(); private bool disposed; - internal PostExtractionHookCallbackWorkerClient(PostExtractionHookInfo hook) + internal PostExtractionHookCallbackWorkerClient(PostExtractionHookInfo hook, int maxProtocolLineBytes = WorkerProtocolLineLimits.MaxLineUtf8Bytes) { this.hook = hook; + this.maxProtocolLineBytes = maxProtocolLineBytes; } internal PostExtractionHookCallbackResult Invoke( @@ -73,14 +76,18 @@ internal PostExtractionHookCallbackResult Invoke( Task sendTask; try { - responseTask = process!.StandardOutput.ReadLineAsync(); + responseTask = BoundedLineReader.ReadLineAsync( + process!.StandardOutput, + maxProtocolLineBytes, + maxProtocolLineBytes, + CancellationToken.None); sendTask = SendRequestAsync(process.StandardInput, requestJson); } catch (Exception ex) { KillWorker(); stopwatch.Stop(); - return Failure($"failed to send worker request: {ex.Message}", stopwatch.ElapsedMilliseconds); + return Failure($"{SafeDiagnosticFormatter.FormatExceptionCategory("worker_protocol_error", ex)} while sending hook callback request.", stopwatch.ElapsedMilliseconds); } if (!WaitForTask(sendTask, waitMilliseconds, out var sendException)) @@ -94,7 +101,7 @@ internal PostExtractionHookCallbackResult Invoke( { KillWorker(); stopwatch.Stop(); - return Failure($"failed to send worker request: {sendException.Message}", stopwatch.ElapsedMilliseconds); + return Failure($"{SafeDiagnosticFormatter.FormatExceptionCategory("worker_protocol_error", sendException)} while sending hook callback request.", stopwatch.ElapsedMilliseconds); } waitMilliseconds = GetRemainingWaitMilliseconds(stopwatch, callbackBudget); @@ -109,7 +116,7 @@ internal PostExtractionHookCallbackResult Invoke( { KillWorker(); stopwatch.Stop(); - return Failure($"failed to read worker response: {responseException.Message}", stopwatch.ElapsedMilliseconds); + return Failure($"{SafeDiagnosticFormatter.FormatExceptionCategory("worker_protocol_error", responseException)} while reading hook callback response.", stopwatch.ElapsedMilliseconds); } if (CallbackBudgetExceeded(stopwatch, callbackBudget)) @@ -138,7 +145,7 @@ internal PostExtractionHookCallbackResult Invoke( catch (JsonException ex) { KillWorker(); - return Failure($"worker returned invalid JSON: {ex.Message}", stopwatch.ElapsedMilliseconds); + return Failure($"{SafeDiagnosticFormatter.FormatExceptionCategory("worker_protocol_error", ex)} while parsing hook callback response.", stopwatch.ElapsedMilliseconds); } if (response == null) @@ -198,7 +205,7 @@ private bool EnsureStarted(out string error) ClearExitedWorker(); stderr = new StringBuilder(); - if (!PostExtractionHookCallbackWorker.TryCreateStartInfo(hook, out var startInfo, out error)) + if (!PostExtractionHookCallbackWorker.TryCreateStartInfo(hook, maxProtocolLineBytes, out var startInfo, out error)) return false; var next = new Process { StartInfo = startInfo, EnableRaisingEvents = true }; @@ -224,7 +231,7 @@ private bool EnsureStarted(out string error) } catch (Exception ex) { - error = $"failed to start worker process: {ex.Message}"; + error = SafeDiagnosticFormatter.FormatExceptionCategory("worker_start_failed", ex); next.Dispose(); return false; } @@ -313,11 +320,8 @@ private static bool CallbackBudgetExceeded(Stopwatch stopwatch, TimeSpan callbac private static string BuildWorkerExitError(Process? process, string stderr, string fallback) { - var exitCodeText = process == null - ? "unknown" - : process.ExitCode.ToString(System.Globalization.CultureInfo.InvariantCulture); - var detail = !string.IsNullOrWhiteSpace(stderr) ? stderr.Trim() : fallback; - return $"worker exited with code {exitCodeText}: {detail}"; + var exitCode = process == null ? (int?)null : process.ExitCode; + return SafeDiagnosticFormatter.FormatWorkerExit("worker_protocol_error", exitCode, fallback); } private static bool WaitForWorkerExit(Process process, int milliseconds) @@ -337,6 +341,7 @@ internal static class PostExtractionHookCallbackWorker { internal const string CommandName = "__cdidx-post-extraction-hook-callback"; internal const int WorkerKillWaitMilliseconds = 5000; + private const string ProtocolMaxLineBytesOption = "--protocol-max-line-bytes"; internal static readonly JsonSerializerOptions JsonOptions = PostExtractionHookCallbackWorkerJsonContext.Default.Options; internal static bool TryRunCommand( @@ -344,7 +349,9 @@ internal static bool TryRunCommand( TextReader input, TextWriter output, TextWriter error, - out int exitCode) + out int exitCode, + int maxProtocolLineCharacters = WorkerProtocolLineLimits.MaxLineCharacters, + int maxProtocolLineUtf8Bytes = WorkerProtocolLineLimits.MaxLineUtf8Bytes) { if (args.Length == 0 || !StringComparer.Ordinal.Equals(args[0], CommandName)) { @@ -352,7 +359,7 @@ internal static bool TryRunCommand( return false; } - exitCode = RunCommand(args, input, output, error); + exitCode = RunCommand(args, input, output, error, maxProtocolLineCharacters, maxProtocolLineUtf8Bytes); return true; } @@ -360,11 +367,25 @@ internal static bool TryCreateStartInfo( PostExtractionHookInfo hook, out ProcessStartInfo startInfo, out string error) + { + return TryCreateStartInfo( + hook, + WorkerProtocolLineLimits.MaxLineUtf8Bytes, + out startInfo, + out error); + } + + internal static bool TryCreateStartInfo( + PostExtractionHookInfo hook, + int maxProtocolLineBytes, + out ProcessStartInfo startInfo, + out string error) { return TryCreateStartInfo( hook, Environment.ProcessPath, ResolveCurrentRunnerAssemblyPath(), + maxProtocolLineBytes, out startInfo, out error); } @@ -375,6 +396,23 @@ internal static bool TryCreateStartInfo( string? runnerAssemblyPath, out ProcessStartInfo startInfo, out string error) + { + return TryCreateStartInfo( + hook, + currentProcessPath, + runnerAssemblyPath, + WorkerProtocolLineLimits.MaxLineUtf8Bytes, + out startInfo, + out error); + } + + internal static bool TryCreateStartInfo( + PostExtractionHookInfo hook, + string? currentProcessPath, + string? runnerAssemblyPath, + int maxProtocolLineBytes, + out ProcessStartInfo startInfo, + out string error) { startInfo = CreateStartInfo(); if (ShouldStartCurrentExecutable(currentProcessPath, runnerAssemblyPath)) @@ -383,6 +421,7 @@ internal static bool TryCreateStartInfo( startInfo.ArgumentList.Add(CommandName); startInfo.ArgumentList.Add(hook.AssemblyPath); startInfo.ArgumentList.Add(hook.TypeName); + AddProtocolLineLimitArguments(startInfo, maxProtocolLineBytes); error = string.Empty; return true; } @@ -399,6 +438,7 @@ internal static bool TryCreateStartInfo( startInfo.ArgumentList.Add(CommandName); startInfo.ArgumentList.Add(hook.AssemblyPath); startInfo.ArgumentList.Add(hook.TypeName); + AddProtocolLineLimitArguments(startInfo, maxProtocolLineBytes); ApplyCurrentRuntimeRollForward(startInfo); error = string.Empty; @@ -427,33 +467,75 @@ internal static void TryKillProcess(Process process) } } - private static int RunCommand(string[] args, TextReader input, TextWriter output, TextWriter error) + private static int RunCommand( + string[] args, + TextReader input, + TextWriter output, + TextWriter error, + int maxProtocolLineCharacters, + int maxProtocolLineUtf8Bytes) { - if (args.Length != 3) + if (!TryResolveProtocolLineLimit( + args, + maxProtocolLineCharacters, + maxProtocolLineUtf8Bytes, + out var resolvedProtocolLineCharacters, + out var resolvedProtocolLineUtf8Bytes, + out var protocolLimitError)) { - error.WriteLine("post-extraction hook callback worker requires assembly path and type name."); + error.WriteLine(protocolLimitError); return 2; } + maxProtocolLineCharacters = resolvedProtocolLineCharacters; + maxProtocolLineUtf8Bytes = resolvedProtocolLineUtf8Bytes; + var hookAssemblyPath = args[1]; var hookTypeName = args[2]; try { IPostExtractionHook? hook = null; - string? requestJson; - while ((requestJson = input.ReadLine()) != null) + while (true) { WorkerResponse response; + WorkerRequest request; + string? requestJson; try { - var request = JsonSerializer.Deserialize(requestJson, JsonOptions) + requestJson = BoundedLineReader.ReadLine(input, maxProtocolLineCharacters, maxProtocolLineUtf8Bytes); + } + catch (BoundedLineLengthException ex) + { + response = new WorkerResponse(null, null, null, SafeDiagnosticFormatter.FormatExceptionCategory("worker_protocol_error", ex)); + output.WriteLine(JsonSerializer.Serialize(response, JsonOptions)); + output.Flush(); + return 1; + } + + if (requestJson is null) + break; + + try + { + request = JsonSerializer.Deserialize(requestJson, JsonOptions) ?? throw new InvalidOperationException("worker request was empty."); + } + catch (Exception ex) + { + response = new WorkerResponse(null, null, null, SafeDiagnosticFormatter.FormatExceptionCategory("worker_protocol_error", ex)); + output.WriteLine(JsonSerializer.Serialize(response, JsonOptions)); + output.Flush(); + continue; + } + + try + { hook ??= CreateHook(hookAssemblyPath, hookTypeName); response = InvokeInsideWorker(hook, request); } catch (Exception ex) { - response = new WorkerResponse(null, null, null, ex.Message); + response = new WorkerResponse(null, null, null, SafeDiagnosticFormatter.FormatExceptionCategory("worker_execution_failed", ex)); } output.WriteLine(JsonSerializer.Serialize(response, JsonOptions)); @@ -464,7 +546,7 @@ private static int RunCommand(string[] args, TextReader input, TextWriter output } catch (Exception ex) { - error.WriteLine(ex.Message); + error.WriteLine(SafeDiagnosticFormatter.FormatExceptionCategory("worker_protocol_error", ex)); return 1; } } @@ -516,7 +598,45 @@ private static WorkerResponse InvokeInsideWorker(IPostExtractionHook hook, Worke Console.SetError(originalError); } - return new WorkerResponse(request.Symbols, request.References, callbackFailure?.Message, null); + return new WorkerResponse( + request.Symbols, + request.References, + callbackFailure is null ? null : SafeDiagnosticFormatter.FormatExceptionCategory("hook_callback_failed", callbackFailure), + null); + } + + private static void AddProtocolLineLimitArguments(ProcessStartInfo startInfo, int maxProtocolLineBytes) + { + startInfo.ArgumentList.Add(ProtocolMaxLineBytesOption); + startInfo.ArgumentList.Add(maxProtocolLineBytes.ToString(CultureInfo.InvariantCulture)); + } + + private static bool TryResolveProtocolLineLimit( + string[] args, + int fallbackMaxProtocolLineCharacters, + int fallbackMaxProtocolLineUtf8Bytes, + out int maxProtocolLineCharacters, + out int maxProtocolLineUtf8Bytes, + out string error) + { + maxProtocolLineCharacters = fallbackMaxProtocolLineCharacters; + maxProtocolLineUtf8Bytes = fallbackMaxProtocolLineUtf8Bytes; + error = string.Empty; + if (args.Length == 3) + return true; + + if (args.Length == 5 + && StringComparer.Ordinal.Equals(args[3], ProtocolMaxLineBytesOption) + && int.TryParse(args[4], NumberStyles.Integer, CultureInfo.InvariantCulture, out var parsed) + && parsed > 0) + { + maxProtocolLineCharacters = parsed; + maxProtocolLineUtf8Bytes = parsed; + return true; + } + + error = $"post-extraction hook callback worker requires assembly path, type name, and optional `{ProtocolMaxLineBytesOption} `."; + return false; } private static string ResolveDotnetHostPath() diff --git a/src/CodeIndex/Indexer/Hooks/PostExtractionHooks.cs b/src/CodeIndex/Indexer/Hooks/PostExtractionHooks.cs index 94297bb133..4113b636fe 100644 --- a/src/CodeIndex/Indexer/Hooks/PostExtractionHooks.cs +++ b/src/CodeIndex/Indexer/Hooks/PostExtractionHooks.cs @@ -54,8 +54,8 @@ private PostExtractionHookRunner(List hooks, TimeSpan this.callbackBudget = callbackBudget; } - public static PostExtractionHookRunner DiscoverDefault() - => Discover(GetDefaultHooksDirectory()); + public static PostExtractionHookRunner DiscoverDefault(long? maxFileSizeBytes = null) + => Discover(GetDefaultHooksDirectory(), maxFileSizeBytes); public static PostExtractionHookDiscoverySnapshot DiscoverDefaultMetadata() => DiscoverMetadata(GetDefaultHooksDirectory()); @@ -81,10 +81,11 @@ public static PostExtractionHookDiscoverySnapshot DiscoverMetadata(string? hooks return new PostExtractionHookDiscoverySnapshot(hooks, runner.Diagnostics, runner.CallbackBudget); } - public static PostExtractionHookRunner Discover(string? hooksDirectory) + public static PostExtractionHookRunner Discover(string? hooksDirectory, long? maxFileSizeBytes = null) { var loaded = new List(); var runner = new PostExtractionHookRunner(loaded, ResolveCallbackBudget()); + var maxProtocolLineBytes = WorkerProtocolLineLimits.ResolveForSourceFileBytes(maxFileSizeBytes); if (string.IsNullOrWhiteSpace(hooksDirectory) || !Directory.Exists(hooksDirectory)) return runner; @@ -135,7 +136,7 @@ public static PostExtractionHookRunner Discover(string? hooksDirectory) loaded.Add(new LoadedPostExtractionHook( info, AssemblyLoadContext.GetLoadContext(type.Assembly), - new PostExtractionHookCallbackWorkerClient(info))); + new PostExtractionHookCallbackWorkerClient(info, maxProtocolLineBytes))); } catch (Exception) { diff --git a/src/CodeIndex/Indexer/Symbols/SymbolExtractionWorker.cs b/src/CodeIndex/Indexer/Symbols/SymbolExtractionWorker.cs index dcd05587e9..a4c7a921ef 100644 --- a/src/CodeIndex/Indexer/Symbols/SymbolExtractionWorker.cs +++ b/src/CodeIndex/Indexer/Symbols/SymbolExtractionWorker.cs @@ -1,4 +1,5 @@ using System.Diagnostics; +using System.Globalization; using System.Reflection; using System.Runtime.Versioning; using System.Text; @@ -17,11 +18,17 @@ internal sealed record SymbolExtractionWorkerResult( internal sealed class SymbolExtractionWorkerClient : IDisposable { + private readonly int maxProtocolLineBytes; private readonly object gate = new(); private Process? process; private StringBuilder stderr = new(); private bool disposed; + internal SymbolExtractionWorkerClient(long? maxFileSizeBytes = null) + { + maxProtocolLineBytes = WorkerProtocolLineLimits.ResolveForSourceFileBytes(maxFileSizeBytes); + } + internal SymbolExtractionWorkerResult Invoke( long fileId, string? lang, @@ -55,14 +62,18 @@ internal SymbolExtractionWorkerResult Invoke( Task sendTask; try { - responseTask = process!.StandardOutput.ReadLineAsync(); + responseTask = BoundedLineReader.ReadLineAsync( + process!.StandardOutput, + maxProtocolLineBytes, + maxProtocolLineBytes, + cancellationToken); sendTask = SendRequestAsync(process.StandardInput, requestJson); } catch (Exception ex) { KillWorker(); stopwatch.Stop(); - return Failure($"failed to send symbol extraction request: {ex.Message}", stopwatch.ElapsedMilliseconds); + return Failure($"{SafeDiagnosticFormatter.FormatExceptionCategory("worker_protocol_error", ex)} while sending symbol extraction request.", stopwatch.ElapsedMilliseconds); } if (!WaitForTask(sendTask, waitMilliseconds, cancellationToken, out var sendException)) @@ -76,7 +87,7 @@ internal SymbolExtractionWorkerResult Invoke( { KillWorker(); stopwatch.Stop(); - return Failure($"failed to send symbol extraction request: {sendException.Message}", stopwatch.ElapsedMilliseconds); + return Failure($"{SafeDiagnosticFormatter.FormatExceptionCategory("worker_protocol_error", sendException)} while sending symbol extraction request.", stopwatch.ElapsedMilliseconds); } waitMilliseconds = GetRemainingWaitMilliseconds(stopwatch, callbackBudget); @@ -91,7 +102,7 @@ internal SymbolExtractionWorkerResult Invoke( { KillWorker(); stopwatch.Stop(); - return Failure($"failed to read symbol extraction response: {responseException.Message}", stopwatch.ElapsedMilliseconds); + return Failure($"{SafeDiagnosticFormatter.FormatExceptionCategory("worker_protocol_error", responseException)} while reading symbol extraction response.", stopwatch.ElapsedMilliseconds); } if (CallbackBudgetExceeded(stopwatch, callbackBudget)) @@ -120,7 +131,7 @@ internal SymbolExtractionWorkerResult Invoke( catch (JsonException ex) { KillWorker(); - return Failure($"worker returned invalid JSON: {ex.Message}", stopwatch.ElapsedMilliseconds); + return Failure($"{SafeDiagnosticFormatter.FormatExceptionCategory("worker_protocol_error", ex)} while parsing symbol extraction response.", stopwatch.ElapsedMilliseconds); } if (response == null) @@ -177,7 +188,7 @@ private bool EnsureStarted(out string error) ClearExitedWorker(); stderr = new StringBuilder(); - if (!SymbolExtractionWorker.TryCreateStartInfo(out var startInfo, out error)) + if (!SymbolExtractionWorker.TryCreateStartInfo(maxProtocolLineBytes, out var startInfo, out error)) return false; var next = new Process { StartInfo = startInfo, EnableRaisingEvents = true }; @@ -203,7 +214,7 @@ private bool EnsureStarted(out string error) } catch (Exception ex) { - error = $"failed to start symbol extraction worker process: {ex.Message}"; + error = SafeDiagnosticFormatter.FormatExceptionCategory("worker_start_failed", ex); next.Dispose(); return false; } @@ -293,11 +304,8 @@ private static bool CallbackBudgetExceeded(Stopwatch stopwatch, TimeSpan callbac private static string BuildWorkerExitError(Process? process, string stderr, string fallback) { - var exitCodeText = process == null - ? "unknown" - : process.ExitCode.ToString(System.Globalization.CultureInfo.InvariantCulture); - var detail = !string.IsNullOrWhiteSpace(stderr) ? stderr.Trim() : fallback; - return $"worker exited with code {exitCodeText}: {detail}"; + var exitCode = process == null ? (int?)null : process.ExitCode; + return SafeDiagnosticFormatter.FormatWorkerExit("worker_protocol_error", exitCode, fallback); } private static bool WaitForWorkerExit(Process process, int milliseconds) @@ -326,6 +334,7 @@ internal static class SymbolExtractionWorker internal const string DelayEnvironmentVariable = "CDIDX_TEST_SYMBOL_EXTRACTION_WORKER_DELAY_MS"; internal const string CompletionPathEnvironmentVariable = "CDIDX_TEST_SYMBOL_EXTRACTION_WORKER_DONE_PATH"; internal const string ConsoleStdoutEnvironmentVariable = "CDIDX_TEST_SYMBOL_EXTRACTION_WORKER_STDOUT"; + private const string ProtocolMaxLineBytesOption = "--protocol-max-line-bytes"; private const int CapturedConsoleMaxChars = 32 * 1024; internal static readonly JsonSerializerOptions JsonOptions = SymbolExtractionWorkerJsonContext.Default.Options; @@ -334,7 +343,9 @@ internal static bool TryRunCommand( TextReader input, TextWriter output, TextWriter error, - out int exitCode) + out int exitCode, + int maxProtocolLineCharacters = WorkerProtocolLineLimits.MaxLineCharacters, + int maxProtocolLineUtf8Bytes = WorkerProtocolLineLimits.MaxLineUtf8Bytes) { if (args.Length == 0 || !StringComparer.Ordinal.Equals(args[0], CommandName)) { @@ -342,15 +353,38 @@ internal static bool TryRunCommand( return false; } - exitCode = RunCommand(args, input, output, error); + exitCode = RunCommand(args, input, output, error, maxProtocolLineCharacters, maxProtocolLineUtf8Bytes); return true; } internal static bool TryCreateStartInfo(out ProcessStartInfo startInfo, out string error) + { + return TryCreateStartInfo( + WorkerProtocolLineLimits.MaxLineUtf8Bytes, + out startInfo, + out error); + } + + internal static bool TryCreateStartInfo(int maxProtocolLineBytes, out ProcessStartInfo startInfo, out string error) { return TryCreateStartInfo( Environment.ProcessPath, ResolveCurrentRunnerAssemblyPath(), + maxProtocolLineBytes, + out startInfo, + out error); + } + + internal static bool TryCreateStartInfo( + string? currentProcessPath, + string? runnerAssemblyPath, + out ProcessStartInfo startInfo, + out string error) + { + return TryCreateStartInfo( + currentProcessPath, + runnerAssemblyPath, + WorkerProtocolLineLimits.MaxLineUtf8Bytes, out startInfo, out error); } @@ -358,6 +392,7 @@ internal static bool TryCreateStartInfo(out ProcessStartInfo startInfo, out stri internal static bool TryCreateStartInfo( string? currentProcessPath, string? runnerAssemblyPath, + int maxProtocolLineBytes, out ProcessStartInfo startInfo, out string error) { @@ -366,6 +401,7 @@ internal static bool TryCreateStartInfo( { startInfo.FileName = currentProcessPath!; startInfo.ArgumentList.Add(CommandName); + AddProtocolLineLimitArguments(startInfo, maxProtocolLineBytes); error = string.Empty; return true; } @@ -380,6 +416,7 @@ internal static bool TryCreateStartInfo( startInfo.FileName = ResolveDotnetHostPath(); startInfo.ArgumentList.Add(runnerAssemblyPath); startInfo.ArgumentList.Add(CommandName); + AddProtocolLineLimitArguments(startInfo, maxProtocolLineBytes); ApplyCurrentRuntimeRollForward(startInfo); error = string.Empty; @@ -437,29 +474,71 @@ internal static void TryKillProcess(Process process) } } - private static int RunCommand(string[] args, TextReader input, TextWriter output, TextWriter error) + private static int RunCommand( + string[] args, + TextReader input, + TextWriter output, + TextWriter error, + int maxProtocolLineCharacters, + int maxProtocolLineUtf8Bytes) { - if (args.Length != 1) - { - error.WriteLine("symbol extraction worker does not accept positional arguments."); + if (!TryResolveProtocolLineLimit( + args, + maxProtocolLineCharacters, + maxProtocolLineUtf8Bytes, + out var resolvedProtocolLineCharacters, + out var resolvedProtocolLineUtf8Bytes, + out var protocolLimitError)) + { + error.WriteLine(protocolLimitError); return 2; } + maxProtocolLineCharacters = resolvedProtocolLineCharacters; + maxProtocolLineUtf8Bytes = resolvedProtocolLineUtf8Bytes; + try { - string? requestJson; - while ((requestJson = input.ReadLine()) != null) + while (true) { WorkerResponse response; + WorkerRequest request; + string? requestJson; try { - var request = JsonSerializer.Deserialize(requestJson, JsonOptions) + requestJson = BoundedLineReader.ReadLine(input, maxProtocolLineCharacters, maxProtocolLineUtf8Bytes); + } + catch (BoundedLineLengthException ex) + { + response = new WorkerResponse(null, SafeDiagnosticFormatter.FormatExceptionCategory("worker_protocol_error", ex), null); + output.WriteLine(JsonSerializer.Serialize(response, JsonOptions)); + output.Flush(); + return 1; + } + + if (requestJson is null) + break; + + try + { + request = JsonSerializer.Deserialize(requestJson, JsonOptions) ?? throw new InvalidOperationException("worker request was empty."); + } + catch (Exception ex) + { + response = new WorkerResponse(null, SafeDiagnosticFormatter.FormatExceptionCategory("worker_protocol_error", ex), null); + output.WriteLine(JsonSerializer.Serialize(response, JsonOptions)); + output.Flush(); + continue; + } + + try + { response = InvokeInsideWorker(request); } catch (Exception ex) { - response = new WorkerResponse(null, ex.Message, null); + response = new WorkerResponse(null, SafeDiagnosticFormatter.FormatExceptionCategory("worker_execution_failed", ex), null); } output.WriteLine(JsonSerializer.Serialize(response, JsonOptions)); @@ -470,7 +549,7 @@ private static int RunCommand(string[] args, TextReader input, TextWriter output } catch (Exception ex) { - error.WriteLine(ex.Message); + error.WriteLine(SafeDiagnosticFormatter.FormatExceptionCategory("worker_protocol_error", ex)); return 1; } } @@ -498,7 +577,7 @@ private static WorkerResponse InvokeInsideWorker(WorkerRequest request) } catch (Exception ex) { - return new WorkerResponse(null, ex.Message, capturedError.GetCapturedText()); + return new WorkerResponse(null, SafeDiagnosticFormatter.FormatExceptionCategory("worker_execution_failed", ex), capturedError.GetCapturedText()); } finally { @@ -517,7 +596,7 @@ private static void WriteConsoleOutputForTestingIfRequested() private static void DelayForTestingIfRequested() { var raw = Environment.GetEnvironmentVariable(DelayEnvironmentVariable); - if (!int.TryParse(raw, System.Globalization.NumberStyles.Integer, System.Globalization.CultureInfo.InvariantCulture, out var milliseconds) + if (!int.TryParse(raw, NumberStyles.Integer, CultureInfo.InvariantCulture, out var milliseconds) || milliseconds <= 0) { return; @@ -529,6 +608,40 @@ private static void DelayForTestingIfRequested() File.WriteAllText(completionPath, "completed"); } + private static void AddProtocolLineLimitArguments(ProcessStartInfo startInfo, int maxProtocolLineBytes) + { + startInfo.ArgumentList.Add(ProtocolMaxLineBytesOption); + startInfo.ArgumentList.Add(maxProtocolLineBytes.ToString(CultureInfo.InvariantCulture)); + } + + private static bool TryResolveProtocolLineLimit( + string[] args, + int fallbackMaxProtocolLineCharacters, + int fallbackMaxProtocolLineUtf8Bytes, + out int maxProtocolLineCharacters, + out int maxProtocolLineUtf8Bytes, + out string error) + { + maxProtocolLineCharacters = fallbackMaxProtocolLineCharacters; + maxProtocolLineUtf8Bytes = fallbackMaxProtocolLineUtf8Bytes; + error = string.Empty; + if (args.Length == 1) + return true; + + if (args.Length == 3 + && StringComparer.Ordinal.Equals(args[1], ProtocolMaxLineBytesOption) + && int.TryParse(args[2], NumberStyles.Integer, CultureInfo.InvariantCulture, out var parsed) + && parsed > 0) + { + maxProtocolLineCharacters = parsed; + maxProtocolLineUtf8Bytes = parsed; + return true; + } + + error = $"symbol extraction worker accepts only `{ProtocolMaxLineBytesOption} `."; + return false; + } + private static string ResolveDotnetHostPath() { var dotnetHostPath = Environment.GetEnvironmentVariable("DOTNET_HOST_PATH"); diff --git a/src/CodeIndex/Mcp/HttpMcpTransport.cs b/src/CodeIndex/Mcp/HttpMcpTransport.cs index d6796d5e64..a2e34324a2 100644 --- a/src/CodeIndex/Mcp/HttpMcpTransport.cs +++ b/src/CodeIndex/Mcp/HttpMcpTransport.cs @@ -129,8 +129,8 @@ internal HttpMcpTransport( FullMode = BoundedChannelFullMode.Wait, AllowSynchronousContinuations = false, }); - if (McpAuthenticationLimits.IsTokenOversized(bearerToken)) - throw new ArgumentException($"Token must not exceed {McpAuthenticationLimits.MaxTokenCharacters.ToString(CultureInfo.InvariantCulture)} characters.", nameof(bearerToken)); + if (bearerToken is { Length: > 0 } && !McpAuthenticationLimits.IsTokenShapeValid(bearerToken)) + throw new ArgumentException(McpAuthenticationLimits.FormatTokenShapeError("Token"), nameof(bearerToken)); _handlerSemaphore = new SemaphoreSlim(_maxConcurrentHandlers, _maxConcurrentHandlers); _listener = new HttpListener(); _listener.Prefixes.Add(prefix); @@ -619,7 +619,7 @@ private async Task TryAuthorizeAsync(PendingRequest request) var header = context.Request.Headers["Authorization"]; if (string.IsNullOrEmpty(header)) { - request.AuthOutcome = "missing"; + request.AuthOutcome = FormatAuthFailureOutcome("missing"); } else if (TryExtractBearerToken(header, out var provided)) { @@ -629,11 +629,11 @@ private async Task TryAuthorizeAsync(PendingRequest request) return true; } - request.AuthOutcome = "wrong-token"; + request.AuthOutcome = FormatAuthFailureOutcome("wrong-token"); } else { - request.AuthOutcome = "wrong-scheme"; + request.AuthOutcome = FormatAuthFailureOutcome("wrong-scheme"); } // RFC 7235 §4.1: 401 responses SHOULD carry a WWW-Authenticate challenge so @@ -647,14 +647,17 @@ private async Task TryAuthorizeAsync(PendingRequest request) return false; } + private static string FormatAuthFailureOutcome(string detailedOutcome) + => McpServer.IsUnsafeDebugEnabled() ? detailedOutcome : "unauthorized"; + private static bool TryExtractBearerToken(string header, out string? token) { token = null; if (header.Length < BearerPrefix.Length || !header.StartsWith(BearerPrefix, StringComparison.OrdinalIgnoreCase)) return false; - var candidate = header.AsSpan(BearerPrefix.Length).Trim(); - if (candidate.Length > McpAuthenticationLimits.MaxTokenCharacters) + var candidate = header.AsSpan(BearerPrefix.Length); + if (!McpAuthenticationLimits.IsTokenShapeValid(candidate)) return true; token = candidate.ToString(); diff --git a/src/CodeIndex/Mcp/McpAuthentication.cs b/src/CodeIndex/Mcp/McpAuthentication.cs index 4fc8bcedbd..4d0f9b8776 100644 --- a/src/CodeIndex/Mcp/McpAuthentication.cs +++ b/src/CodeIndex/Mcp/McpAuthentication.cs @@ -51,6 +51,23 @@ internal static class McpAuthenticationLimits internal static bool IsTokenOversized(string? token) => token is { Length: > MaxTokenCharacters }; + + internal static bool IsTokenShapeValid(ReadOnlySpan token) + { + if (token.Length == 0 || token.Length > MaxTokenCharacters) + return false; + + foreach (var ch in token) + { + if (char.IsWhiteSpace(ch) || char.IsControl(ch)) + return false; + } + + return true; + } + + internal static string FormatTokenShapeError(string source) + => $"{source} must be 1 to {MaxTokenCharacters} characters and must not contain whitespace or control characters."; } /// @@ -134,10 +151,8 @@ public sealed class TokenMcpAuthenticator : IMcpAuthenticator public TokenMcpAuthenticator(string expectedToken) { - if (string.IsNullOrEmpty(expectedToken)) - throw new ArgumentException("Token must not be empty", nameof(expectedToken)); - if (McpAuthenticationLimits.IsTokenOversized(expectedToken)) - throw new ArgumentException($"Token must not exceed {McpAuthenticationLimits.MaxTokenCharacters} characters.", nameof(expectedToken)); + if (!McpAuthenticationLimits.IsTokenShapeValid(expectedToken)) + throw new ArgumentException(McpAuthenticationLimits.FormatTokenShapeError("Token"), nameof(expectedToken)); _expectedTokenHash = SHA256.HashData(Encoding.UTF8.GetBytes(expectedToken)); } @@ -203,8 +218,10 @@ public static class McpAuthenticatorFactory public static IMcpAuthenticator FromEnvironment() { var token = Environment.GetEnvironmentVariable(AuthTokenEnvVar); - if (string.IsNullOrWhiteSpace(token)) + if (string.IsNullOrEmpty(token)) return LocalStdioAuthenticator.Instance; + if (!McpAuthenticationLimits.IsTokenShapeValid(token)) + throw new FormatException(McpAuthenticationLimits.FormatTokenShapeError(AuthTokenEnvVar)); return new TokenMcpAuthenticator(token); } } diff --git a/src/CodeIndex/Mcp/McpServer.cs b/src/CodeIndex/Mcp/McpServer.cs index 8403e97c58..4cee9bf16b 100644 --- a/src/CodeIndex/Mcp/McpServer.cs +++ b/src/CodeIndex/Mcp/McpServer.cs @@ -554,6 +554,13 @@ internal async Task RunAsync(IMcpTransport transport, CancellationToken cancella FlushDeferredFrameLogs(); break; } + catch (BoundedLineLengthException ex) + { + BeginDeferredFrameLogs(); + await WriteFrameSafelyAsync(transport, BuildOversizedLineErrorResponse(ex), loopToken).ConfigureAwait(false); + FlushDeferredFrameLogs(); + break; + } } } finally @@ -603,6 +610,22 @@ private async Task RunConcurrentFrameLoopAsync(IMcpTransport transport, Cancella } break; } + catch (BoundedLineLengthException ex) + { + await Task.WhenAll(tasks).ConfigureAwait(false); + await writeGate.WaitAsync(loopToken).ConfigureAwait(false); + try + { + BeginDeferredFrameLogs(); + await WriteFrameSafelyAsync(transport, BuildOversizedLineErrorResponse(ex), loopToken).ConfigureAwait(false); + FlushDeferredFrameLogs(); + } + finally + { + writeGate.Release(); + } + break; + } if (frame == null) break; @@ -873,6 +896,19 @@ private string BuildInvalidUtf8ParseErrorResponse(DecoderFallbackException ex) internal static string BuildInvalidUtf8ErrorLog(string detail) => $"[cdidx-mcp] JSON parse error: invalid UTF-8 input ({detail}). Send one UTF-8 JSON-RPC object per line; reject or re-encode malformed bytes before retrying."; + private string BuildOversizedLineErrorResponse(BoundedLineLengthException ex) + => BuildOversizedLineErrorResponse(ex.CharactersRead, ex.Utf8BytesRead); + + private string BuildOversizedLineErrorResponse(int charactersRead, int utf8BytesRead) + { + DeferFrameLog(BuildOversizedMessageLog(charactersRead, utf8BytesRead)); + var errorResponse = CreateErrorResponse(hasId: true, id: null, code: -32700, message: "Message too large", + category: McpErrorEnvelope.CategoryMessageTooLarge, + suggestion: $"JSON-RPC frame exceeds the {MaxLineCharacterCount} character or {MaxLineByteLength} byte cap. Split the request into smaller calls or use `batch_query` with smaller slots.", + retrySafe: false); + return errorResponse.ToJsonString(_jsonOptions); + } + /// /// Process one MCP JSON-RPC frame and return the wire-ready response string (or null when /// the request was a notification or otherwise yields no response). This is the @@ -892,14 +928,7 @@ internal static string BuildInvalidUtf8ErrorLog(string detail) // メモリ枯渇を防ぐため巨大メッセージを拒否 var byteLength = Encoding.UTF8.GetByteCount(line); if (line.Length > MaxLineCharacterCount || byteLength > MaxLineByteLength) - { - DeferFrameLog(BuildOversizedMessageLog(line.Length, byteLength)); - var errorResponse = CreateErrorResponse(hasId: true, id: null, code: -32700, message: "Message too large", - category: McpErrorEnvelope.CategoryMessageTooLarge, - suggestion: $"JSON-RPC frame exceeds the {MaxLineCharacterCount} character or {MaxLineByteLength} byte cap. Split the request into smaller calls or use `batch_query` with smaller slots.", - retrySafe: false); - return errorResponse.ToJsonString(_jsonOptions); - } + return BuildOversizedLineErrorResponse(line.Length, byteLength); JsonNode? request = null; var responseHasId = true; @@ -2725,7 +2754,7 @@ JsonObject CreateUnknownToolResponseForMetrics() // パスや索引内容が漏れる（#1530）。 DeferFrameLog(() => { - WriteMcpLogLine(BuildToolErrorLog(toolName, ex.Message)); + WriteMcpLogLine(BuildToolErrorLog(toolName, ex)); Database.DbDebug.DumpToStderr(ex); }); metricsError = ex.GetType().Name; @@ -3259,8 +3288,21 @@ internal static string BuildResponseSerializationErrorLog(string detail) => 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 ({BoundToolNameForDisplay(toolName).Text}): {detail}. Fix the tool arguments, refresh the index if needed, then retry."; + internal static string BuildToolErrorLog(string toolName, Exception ex) => + $"[cdidx-mcp] Tool error ({BoundToolNameForDisplay(toolName).Text}): {BuildSanitizedExceptionLogDetail(ex)}. Fix the tool arguments, refresh the index if needed, then retry."; + + internal static string BuildSanitizedExceptionLogDetail(Exception ex) + { + var exceptionType = McpBoundedText.ForDisplay(ex.GetType().Name).Text; + if (ex is CodeIndexException codeIndexEx) + { + var code = McpBoundedText.ForDisplay(codeIndexEx.Code).Text; + var category = McpBoundedText.ForDisplay(codeIndexEx.Category).Text; + return $"{exceptionType} code={code} category={category}{BuildPathFragment(codeIndexEx)}{BuildHintFragment(codeIndexEx)}"; + } + + return exceptionType; + } internal static string BuildClientResponseTooLargeLog(string member, int bytesWritten) => $"[cdidx-mcp] Client response {member} exceeded the server byte limit ({bytesWritten} > {MaxClientResponseJsonBytes}); rejecting without retaining the payload."; diff --git a/src/CodeIndex/Mcp/McpToolHandlers.cs b/src/CodeIndex/Mcp/McpToolHandlers.cs index 33498999d0..e5172cb937 100644 --- a/src/CodeIndex/Mcp/McpToolHandlers.cs +++ b/src/CodeIndex/Mcp/McpToolHandlers.cs @@ -1408,6 +1408,12 @@ private static bool TryReadReferenceRankMode(JsonNode? args, out ReferenceRankMo return false; } + private static string FormatLiteralSearchQueryLimitError() + => $"literal search query is too long; maximum is {DbReader.MaxLiteralSearchQueryLength} characters. Split generated input into smaller queries."; + + private static string FormatSearchGuardCandidateLimitError(SearchGuardCandidateLimitException ex) + => $"guarded search is too broad: inspected the maximum {ex.CandidateLimit} candidate chunks before satisfying the requested page (limit {ex.RequestedLimit}, offset {ex.RequestedOffset}). Narrow the search with more specific query text, lang/path filters, or a smaller cursor offset."; + private JsonNode ExecuteSearch(JsonNode? id, JsonNode? args) { if (!TryReadRequiredStringParameter(args, "query", out var query, out var requiredError)) @@ -1461,13 +1467,13 @@ private JsonNode ExecuteSearch(JsonNode? id, JsonNode? args) { countResults = reader.Search(query, MaxLimit, lang, rawQuery, pathPatterns, excludePaths, excludeTests, deduplicate, since, exact, prefix, guardFilters: guardFilters, guardWindow: guardWindow); } - catch (SearchQueryLimitException ex) + catch (SearchQueryLimitException) { - return CreateToolErrorResponse(id, ex.Message); + return CreateToolErrorResponse(id, FormatLiteralSearchQueryLimitError()); } catch (SearchGuardCandidateLimitException ex) { - return CreateToolErrorResponse(id, $"guarded search is too broad: {ex.Message} Narrow the search with more specific query text, lang/path filters, or a smaller cursor offset."); + return CreateToolErrorResponse(id, FormatSearchGuardCandidateLimitError(ex)); } var truncatedCount = countResults.Count >= MaxLimit; var payload = BuildCountOnlyPayload(countResults.Count, truncatedCount ? null : countResults.Count, truncatedCount, countResults, result => result.Path); @@ -1489,13 +1495,13 @@ private JsonNode ExecuteSearch(JsonNode? id, JsonNode? args) { results = reader.Search(query, FetchLimitForEnvelope(limit), lang, rawQuery, pathPatterns, excludePaths, excludeTests, deduplicate, since, exact, prefix, cursor: cursor, guardFilters: guardFilters, guardWindow: guardWindow); } - catch (SearchQueryLimitException ex) + catch (SearchQueryLimitException) { - return CreateToolErrorResponse(id, ex.Message); + return CreateToolErrorResponse(id, FormatLiteralSearchQueryLimitError()); } catch (SearchGuardCandidateLimitException ex) { - return CreateToolErrorResponse(id, $"guarded search is too broad: {ex.Message} Narrow the search with more specific query text, lang/path filters, or a smaller cursor offset."); + return CreateToolErrorResponse(id, FormatSearchGuardCandidateLimitError(ex)); } var ftsDiagnostics = DbReader.AnalyzeFtsQuery(query, rawQuery, prefix, lang); var truncated = TrimToRequestedLimit(results, limit); @@ -2828,7 +2834,7 @@ private JsonNode ExecuteFindInFile(JsonNode? id, JsonNode? args) } catch (Exception ex) when (regex && (ex is ArgumentException || ex is RegexMatchTimeoutException)) { - return CreateToolErrorResponse(id, $"invalid regular expression: {ex.Message}"); + return CreateToolErrorResponse(id, "invalid regular expression. Check regex syntax and retry."); } var structured = new JsonObject { @@ -3245,7 +3251,7 @@ void AppendRateLimitedSlot(int requestIndex, string? slotId, string? toolName, J // in stderr instead of the batch_query response. DeferFrameLog(() => { - WriteMcpLogLine(BuildToolErrorLog(toolName, ex.Message)); + WriteMcpLogLine(BuildToolErrorLog(toolName, ex)); Database.DbDebug.DumpToStderr(ex); }); var classification = McpErrorEnvelope.ClassifyException(ex); @@ -4245,7 +4251,7 @@ private async Task ExecuteIndexAsync(JsonNode? id, JsonNode? args, Jso GitHelper.ResolveIgnoreCase(projectPath, requestToken), GitHelper.TryGetRepositoryRoot(projectPath, requestToken) ?? Path.GetFullPath(projectPath), maxFileBytes); - using var postExtractionHooks = PostExtractionHookRunner.DiscoverDefault(); + using var postExtractionHooks = PostExtractionHookRunner.DiscoverDefault(maxFileBytes); var currentHotspotFamilyMarkerFingerprints = GetHotspotFamilyMarkerFingerprints(indexer, requestToken); var currentCSharpSymbolNameContractVersion = DbContext.CSharpSymbolNameContractVersion.ToString(System.Globalization.CultureInfo.InvariantCulture); var csharpSymbolNameContractMatchesCurrent = priorCSharpSymbolNameContractVersion == currentCSharpSymbolNameContractVersion; @@ -4857,7 +4863,7 @@ private JsonNode ExecuteBackfillFold(JsonNode? id, JsonNode? args, JsonNode? pro { DeferFrameLog(() => { - WriteMcpLogLine(BuildToolErrorLog("backfill_fold", ex.Message)); + WriteMcpLogLine(BuildToolErrorLog("backfill_fold", ex)); Database.DbDebug.DumpToStderr(ex); }); var classification = McpErrorEnvelope.ClassifyException(ex); @@ -5439,7 +5445,7 @@ private static bool TryProbeCdidxDirectoryWritable(string cdidxDir, out string? } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { - error = $"Cannot write to .cdidx directory {cdidxDir}; check directory ownership, permissions, and read-only mounts. {ex.Message}"; + error = $"Cannot write to .cdidx directory {cdidxDir}; check directory ownership, permissions, and read-only mounts."; return false; } finally diff --git a/src/CodeIndex/Mcp/StdioMcpTransport.cs b/src/CodeIndex/Mcp/StdioMcpTransport.cs index d489121c2f..65fdc94bdc 100644 --- a/src/CodeIndex/Mcp/StdioMcpTransport.cs +++ b/src/CodeIndex/Mcp/StdioMcpTransport.cs @@ -16,6 +16,8 @@ internal sealed class StdioMcpTransport : IMcpTransport private readonly Stream _stdout; private readonly StreamReader _reader; private readonly StreamWriter _writer; + private readonly int _maxLineCharacters; + private readonly int _maxLineUtf8Bytes; private bool _disposed; public StdioMcpTransport(int bufferSize) @@ -23,10 +25,17 @@ public StdioMcpTransport(int bufferSize) { } - internal StdioMcpTransport(Stream stdin, Stream stdout, int bufferSize) + internal StdioMcpTransport( + Stream stdin, + Stream stdout, + int bufferSize, + int maxLineCharacters = McpServer.MaxLineCharacterCount, + int maxLineUtf8Bytes = McpServer.MaxLineByteLength) { _stdin = stdin; _stdout = stdout; + _maxLineCharacters = maxLineCharacters; + _maxLineUtf8Bytes = maxLineUtf8Bytes; _reader = new StreamReader( _stdin, new UTF8Encoding(encoderShouldEmitUTF8Identifier: false, throwOnInvalidBytes: true), @@ -47,12 +56,11 @@ internal StdioMcpTransport(Stream stdin, Stream stdout, int bufferSize) public async Task ReadFrameAsync(CancellationToken cancellationToken) { ObjectDisposedException.ThrowIf(_disposed, this); - // ReadLineAsync's CancellationToken overload was added in .NET 7; the legacy overload - // remains the call shape used by the existing MCP loop, so we keep it here too and - // honour cancellation only when the writer fails. Stdin closure is the canonical exit. - // ReadLineAsync の CancellationToken 版は .NET 7 で追加されたが、既存ループと同じ - // 呼び出し形を使い、stdin クローズを正規の終了経路として保つ。 - var line = await _reader.ReadLineAsync(cancellationToken).ConfigureAwait(false); + var line = await BoundedLineReader.ReadLineAsync( + _reader, + _maxLineCharacters, + _maxLineUtf8Bytes, + cancellationToken).ConfigureAwait(false); return line; } diff --git a/src/CodeIndex/SafeDiagnosticFormatter.cs b/src/CodeIndex/SafeDiagnosticFormatter.cs new file mode 100644 index 0000000000..a05f4d4614 --- /dev/null +++ b/src/CodeIndex/SafeDiagnosticFormatter.cs @@ -0,0 +1,47 @@ +using System.Text; + +namespace CodeIndex; + +internal static class SafeDiagnosticFormatter +{ + private const int MaxCategoryCharacters = 64; + private const int MaxExceptionTypeCharacters = 128; + private const string TruncationMarker = "..."; + + internal static string FormatExceptionCategory(string category, Exception ex) + => FormatCategoryType(category, ex.GetType().Name); + + internal static string FormatCategoryType(string category, string typeName) + => $"{BoundToken(category, MaxCategoryCharacters)}: {BoundToken(typeName, MaxExceptionTypeCharacters)}"; + + internal static string FormatWorkerExit(string category, int? exitCode, string fallback) + { + var safeCategory = BoundToken(category, MaxCategoryCharacters); + var safeFallback = BoundToken(fallback, MaxExceptionTypeCharacters); + return exitCode.HasValue + ? $"{safeCategory}: worker exited with code {exitCode.Value}. {safeFallback}." + : $"{safeCategory}: worker exited before the exit code was available. {safeFallback}."; + } + + private static string BoundToken(string? value, int maxCharacters) + { + if (string.IsNullOrWhiteSpace(value)) + return "unknown"; + + var builder = new StringBuilder(Math.Min(value.Length, maxCharacters)); + foreach (var ch in value) + { + if (builder.Length >= maxCharacters) + break; + + builder.Append(char.IsControl(ch) || char.IsWhiteSpace(ch) ? '_' : ch); + } + + var bounded = builder.ToString().Trim('_'); + if (bounded.Length == 0) + bounded = "unknown"; + if (value.Length > maxCharacters && bounded.Length + TruncationMarker.Length <= maxCharacters) + bounded += TruncationMarker; + return bounded; + } +} diff --git a/tests/CodeIndex.Tests/HttpMcpTransportTests.cs b/tests/CodeIndex.Tests/HttpMcpTransportTests.cs index 6e5193cbde..e60751dbb4 100644 --- a/tests/CodeIndex.Tests/HttpMcpTransportTests.cs +++ b/tests/CodeIndex.Tests/HttpMcpTransportTests.cs @@ -169,20 +169,20 @@ public async Task HttpTransport_RequestLogger_RecordsMethodStatusDurationAndAuth Assert.Equal(3, snapshot.Length); // Request logging can be observed from independently handled HTTP requests in any order. - var missingPost = Assert.Single(snapshot, record => - record.AuthOutcome == "missing" && + var unauthorizedPost = Assert.Single(snapshot, record => + record.AuthOutcome == "unauthorized" && record.StatusCode == (int)HttpStatusCode.Unauthorized && record.Method == "POST"); - Assert.Equal("/", missingPost.Path); - Assert.Null(missingPost.RequestId); - Assert.True(missingPost.DurationMs >= 0); - Assert.False(string.IsNullOrWhiteSpace(missingPost.CorrelationId)); - Assert.False(string.IsNullOrWhiteSpace(missingPost.RemotePeer)); - - var missingGet = Assert.Single(snapshot, record => - record.AuthOutcome == "missing" && + Assert.Equal("/", unauthorizedPost.Path); + Assert.Null(unauthorizedPost.RequestId); + Assert.True(unauthorizedPost.DurationMs >= 0); + Assert.False(string.IsNullOrWhiteSpace(unauthorizedPost.CorrelationId)); + Assert.False(string.IsNullOrWhiteSpace(unauthorizedPost.RemotePeer)); + + var unauthorizedGet = Assert.Single(snapshot, record => + record.AuthOutcome == "unauthorized" && record.Method == "GET"); - Assert.Equal((int)HttpStatusCode.Unauthorized, missingGet.StatusCode); + Assert.Equal((int)HttpStatusCode.Unauthorized, unauthorizedGet.StatusCode); var okPost = Assert.Single(snapshot, record => record.AuthOutcome == "ok" && @@ -860,6 +860,23 @@ public async Task HttpTransport_BearerToken_RejectsWrongToken() Assert.Contains(response.Headers.WwwAuthenticate, h => h.Scheme.Equals("Bearer", StringComparison.OrdinalIgnoreCase)); } + [Fact] + public async Task HttpTransport_BearerToken_RejectsWhitespacePaddedHeader_Issue3505() + { + const string token = "s3cret-token"; + await using var harness = await McpHttpHarness.StartAsync(_dbPath, bearerToken: token); + + using var client = new HttpClient(); + using var request = new HttpRequestMessage(HttpMethod.Post, harness.Endpoint) + { + Content = new StringContent("""{"jsonrpc":"2.0","id":1,"method":"ping"}""", Encoding.UTF8, "application/json"), + }; + request.Headers.TryAddWithoutValidation("Authorization", "Bearer " + token); + using var response = await client.SendAsync(request); + + Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode); + } + [Fact] public async Task HttpTransport_BearerToken_RejectsOversizedHeaderBeforeHashing() { @@ -877,6 +894,28 @@ public async Task HttpTransport_BearerToken_RejectsOversizedHeaderBeforeHashing( using var response = await client.SendAsync(request); + Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode); + var record = Assert.Single(await WaitForRequestLogRecordsAsync(records, 1)); + Assert.Equal("unauthorized", record.AuthOutcome); + } + + [Fact] + public async Task HttpTransport_RequestLogger_DetailedAuthOutcomeRequiresUnsafeDebug_Issue3469() + { + using var env = EnvironmentVariableScope.Capture(McpServer.DebugEnvironmentVariable); + env.Set(McpServer.DebugEnvironmentVariable, "unsafe"); + var records = new ConcurrentQueue(); + await using var harness = await McpHttpHarness.StartAsync(_dbPath, bearerToken: "token", requestLogger: records.Enqueue); + + using var client = new HttpClient(); + using var request = new HttpRequestMessage(HttpMethod.Post, harness.Endpoint) + { + Content = new StringContent("""{"jsonrpc":"2.0","id":1,"method":"ping"}""", Encoding.UTF8, "application/json"), + }; + request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", "wrong-token"); + + using var response = await client.SendAsync(request); + Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode); var record = Assert.Single(await WaitForRequestLogRecordsAsync(records, 1)); Assert.Equal("wrong-token", record.AuthOutcome); diff --git a/tests/CodeIndex.Tests/IndexCommandRunnerTests.cs b/tests/CodeIndex.Tests/IndexCommandRunnerTests.cs index 94477f5da4..1236428ae7 100644 --- a/tests/CodeIndex.Tests/IndexCommandRunnerTests.cs +++ b/tests/CodeIndex.Tests/IndexCommandRunnerTests.cs @@ -1,4 +1,5 @@ using System.Diagnostics; +using System.Globalization; using System.Text.Json; using System.Text.RegularExpressions; using System.Runtime.Versioning; @@ -308,6 +309,102 @@ public void SymbolExtractionWorker_CapturesStdoutAndForwardsStderrDiagnostics() } } + [Fact] + public void SymbolExtractionWorker_InvalidRequestJsonDoesNotEchoParserMessage_Issue3425() + { + const string secret = "SECRET_SYMBOL_WORKER_3425"; + using var input = new StringReader("{\"Content\":\"" + secret + "\",\n"); + using var output = new StringWriter(); + using var error = new StringWriter(); + + var handled = SymbolExtractionWorker.TryRunCommand( + [SymbolExtractionWorker.CommandName], + input, + output, + error, + out var exitCode); + + Assert.True(handled); + Assert.Equal(0, exitCode); + Assert.Equal(string.Empty, error.ToString()); + using var document = JsonDocument.Parse(output.ToString()); + var workerError = document.RootElement.GetProperty("WorkerError").GetString(); + Assert.Equal("worker_protocol_error: JsonException", workerError); + Assert.DoesNotContain(secret, output.ToString(), StringComparison.Ordinal); + } + + [Fact] + public void SymbolExtractionWorker_OversizedRequestLineReturnsProtocolError_Issue3506() + { + using var input = new StringReader("abcdef\n"); + using var output = new StringWriter(); + using var error = new StringWriter(); + + var handled = SymbolExtractionWorker.TryRunCommand( + [SymbolExtractionWorker.CommandName], + input, + output, + error, + out var exitCode, + maxProtocolLineCharacters: 5, + maxProtocolLineUtf8Bytes: 100); + + Assert.True(handled); + Assert.Equal(1, exitCode); + Assert.Equal(string.Empty, error.ToString()); + using var document = JsonDocument.Parse(output.ToString()); + var workerError = document.RootElement.GetProperty("WorkerError").GetString(); + Assert.Equal("worker_protocol_error: BoundedLineLengthException", workerError); + } + + [Fact] + public void PostExtractionHookCallbackWorker_InvalidRequestJsonDoesNotEchoParserMessage_Issue3425() + { + const string secret = "SECRET_HOOK_WORKER_3425"; + using var input = new StringReader("{\"Callback\":\"" + secret + "\",\n"); + using var output = new StringWriter(); + using var error = new StringWriter(); + + var handled = PostExtractionHookCallbackWorker.TryRunCommand( + [PostExtractionHookCallbackWorker.CommandName, "/tmp/demo-hook.dll", "Demo.Hook"], + input, + output, + error, + out var exitCode); + + Assert.True(handled); + Assert.Equal(0, exitCode); + Assert.Equal(string.Empty, error.ToString()); + using var document = JsonDocument.Parse(output.ToString()); + var workerError = document.RootElement.GetProperty("WorkerError").GetString(); + Assert.Equal("worker_protocol_error: JsonException", workerError); + Assert.DoesNotContain(secret, output.ToString(), StringComparison.Ordinal); + } + + [Fact] + public void PostExtractionHookCallbackWorker_OversizedRequestLineReturnsProtocolError_Issue3506() + { + using var input = new StringReader("abcdef\n"); + using var output = new StringWriter(); + using var error = new StringWriter(); + + var handled = PostExtractionHookCallbackWorker.TryRunCommand( + [PostExtractionHookCallbackWorker.CommandName, "/tmp/demo-hook.dll", "Demo.Hook"], + input, + output, + error, + out var exitCode, + maxProtocolLineCharacters: 5, + maxProtocolLineUtf8Bytes: 100); + + Assert.True(handled); + Assert.Equal(1, exitCode); + Assert.Equal(string.Empty, error.ToString()); + using var document = JsonDocument.Parse(output.ToString()); + var workerError = document.RootElement.GetProperty("WorkerError").GetString(); + Assert.Equal("worker_protocol_error: BoundedLineLengthException", workerError); + } + [Fact] public void SymbolExtractionWorker_StartInfo_UsesCurrentCdidxExecutableWhenAvailable() { @@ -321,7 +418,13 @@ public void SymbolExtractionWorker_StartInfo_UsesCurrentCdidxExecutableWhenAvail Assert.True(created, error); Assert.Equal(currentProcessPath, startInfo.FileName); - Assert.Equal([SymbolExtractionWorker.CommandName], startInfo.ArgumentList); + Assert.Equal( + [ + SymbolExtractionWorker.CommandName, + "--protocol-max-line-bytes", + WorkerProtocolLineLimits.MaxLineUtf8Bytes.ToString(CultureInfo.InvariantCulture), + ], + startInfo.ArgumentList); Assert.True(startInfo.RedirectStandardInput); Assert.True(startInfo.RedirectStandardOutput); Assert.True(startInfo.RedirectStandardError); @@ -341,7 +444,39 @@ public void SymbolExtractionWorker_StartInfo_UsesFrameworkDependentDllWhenProces Assert.True(created, error); Assert.NotEqual(currentProcessPath, startInfo.FileName); - Assert.Equal([runnerAssemblyPath, SymbolExtractionWorker.CommandName], startInfo.ArgumentList); + Assert.Equal( + [ + runnerAssemblyPath, + SymbolExtractionWorker.CommandName, + "--protocol-max-line-bytes", + WorkerProtocolLineLimits.MaxLineUtf8Bytes.ToString(CultureInfo.InvariantCulture), + ], + startInfo.ArgumentList); + } + + [Fact] + public void SymbolExtractionWorker_StartInfo_RaisesProtocolLimitForLargeFileCap_Issue3506() + { + const long maxFileSizeBytes = 50L * 1024L * 1024L; + var protocolLimit = WorkerProtocolLineLimits.ResolveForSourceFileBytes(maxFileSizeBytes); + var currentProcessPath = Path.Combine(Path.GetTempPath(), OperatingSystem.IsWindows() ? "cdidx.exe" : "cdidx"); + + var created = SymbolExtractionWorker.TryCreateStartInfo( + currentProcessPath, + runnerAssemblyPath: string.Empty, + protocolLimit, + out var startInfo, + out var error); + + Assert.True(created, error); + Assert.True(protocolLimit > WorkerProtocolLineLimits.MaxLineUtf8Bytes); + Assert.Equal( + [ + SymbolExtractionWorker.CommandName, + "--protocol-max-line-bytes", + protocolLimit.ToString(CultureInfo.InvariantCulture), + ], + startInfo.ArgumentList); } [Fact] @@ -362,7 +497,15 @@ public void PostExtractionHookCallbackWorker_StartInfo_UsesCurrentCdidxExecutabl Assert.True(created, error); Assert.Equal(currentProcessPath, startInfo.FileName); - Assert.Equal([PostExtractionHookCallbackWorker.CommandName, hook.AssemblyPath, hook.TypeName], startInfo.ArgumentList); + Assert.Equal( + [ + PostExtractionHookCallbackWorker.CommandName, + hook.AssemblyPath, + hook.TypeName, + "--protocol-max-line-bytes", + WorkerProtocolLineLimits.MaxLineUtf8Bytes.ToString(CultureInfo.InvariantCulture), + ], + startInfo.ArgumentList); Assert.True(startInfo.RedirectStandardInput); Assert.True(startInfo.RedirectStandardOutput); Assert.True(startInfo.RedirectStandardError); @@ -387,7 +530,48 @@ public void PostExtractionHookCallbackWorker_StartInfo_UsesFrameworkDependentDll Assert.True(created, error); Assert.NotEqual(currentProcessPath, startInfo.FileName); - Assert.Equal([runnerAssemblyPath, PostExtractionHookCallbackWorker.CommandName, hook.AssemblyPath, hook.TypeName], startInfo.ArgumentList); + Assert.Equal( + [ + runnerAssemblyPath, + PostExtractionHookCallbackWorker.CommandName, + hook.AssemblyPath, + hook.TypeName, + "--protocol-max-line-bytes", + WorkerProtocolLineLimits.MaxLineUtf8Bytes.ToString(CultureInfo.InvariantCulture), + ], + startInfo.ArgumentList); + } + + [Fact] + public void PostExtractionHookCallbackWorker_StartInfo_RaisesProtocolLimitForLargeFileCap_Issue3506() + { + const long maxFileSizeBytes = 50L * 1024L * 1024L; + var protocolLimit = WorkerProtocolLineLimits.ResolveForSourceFileBytes(maxFileSizeBytes); + var hook = new PostExtractionHookInfo( + "demo", + Path.Combine(Path.GetTempPath(), "demo-hook.dll"), + "Demo.Hook"); + var currentProcessPath = Path.Combine(Path.GetTempPath(), OperatingSystem.IsWindows() ? "cdidx.exe" : "cdidx"); + + var created = PostExtractionHookCallbackWorker.TryCreateStartInfo( + hook, + currentProcessPath, + runnerAssemblyPath: string.Empty, + protocolLimit, + out var startInfo, + out var error); + + Assert.True(created, error); + Assert.True(protocolLimit > WorkerProtocolLineLimits.MaxLineUtf8Bytes); + Assert.Equal( + [ + PostExtractionHookCallbackWorker.CommandName, + hook.AssemblyPath, + hook.TypeName, + "--protocol-max-line-bytes", + protocolLimit.ToString(CultureInfo.InvariantCulture), + ], + startInfo.ArgumentList); } [SkipOnMacOsArm64Fact] diff --git a/tests/CodeIndex.Tests/McpServerTests.cs b/tests/CodeIndex.Tests/McpServerTests.cs index 41c197455c..8a87b4f859 100644 --- a/tests/CodeIndex.Tests/McpServerTests.cs +++ b/tests/CodeIndex.Tests/McpServerTests.cs @@ -2197,8 +2197,8 @@ public void TokenAuthenticator_OversizedTokenInCtor_RejectedBeforeHashing() public void McpAuthenticatorFactory_NoEnv_ReturnsLocalStdio() { // FromEnvironment() must default to permissive stdio when the env var is unset or - // whitespace, so unconfigured installs preserve the historical behaviour. - // 環境変数が未設定 or 空白の場合は permissive stdio に fallback し、未設定インストールの + // empty, so unconfigured installs preserve the historical behaviour. + // 環境変数が未設定 or 空文字の場合は permissive stdio に fallback し、未設定インストールの // 従来動作を維持する。 var previous = Environment.GetEnvironmentVariable(McpAuthenticatorFactory.AuthTokenEnvVar); try @@ -2206,7 +2206,7 @@ public void McpAuthenticatorFactory_NoEnv_ReturnsLocalStdio() Environment.SetEnvironmentVariable(McpAuthenticatorFactory.AuthTokenEnvVar, null); Assert.IsType(McpAuthenticatorFactory.FromEnvironment()); - Environment.SetEnvironmentVariable(McpAuthenticatorFactory.AuthTokenEnvVar, " "); + Environment.SetEnvironmentVariable(McpAuthenticatorFactory.AuthTokenEnvVar, string.Empty); Assert.IsType(McpAuthenticatorFactory.FromEnvironment()); } finally @@ -2215,6 +2215,30 @@ public void McpAuthenticatorFactory_NoEnv_ReturnsLocalStdio() } } + [Fact] + public void McpAuthenticatorFactory_WhitespaceTokenIsRejected_Issue3505() + { + var previous = Environment.GetEnvironmentVariable(McpAuthenticatorFactory.AuthTokenEnvVar); + try + { + Environment.SetEnvironmentVariable(McpAuthenticatorFactory.AuthTokenEnvVar, " token"); + + var ex = Assert.Throws(McpAuthenticatorFactory.FromEnvironment); + Assert.Contains(McpAuthenticatorFactory.AuthTokenEnvVar, ex.Message, StringComparison.Ordinal); + } + finally + { + Environment.SetEnvironmentVariable(McpAuthenticatorFactory.AuthTokenEnvVar, previous); + } + } + + [Fact] + public void TokenAuthenticator_ConfiguredWhitespaceTokenIsRejected_Issue3505() + { + var ex = Assert.Throws(() => new TokenMcpAuthenticator("token ")); + Assert.Contains("whitespace", ex.Message, StringComparison.Ordinal); + } + [Fact] public void TokenAuthenticator_NonStringMethod_ReturnsUnauthorized() { @@ -2434,9 +2458,9 @@ public void BuildResponseWriteErrorLog_IdentifiesResponseWriteStage() [Fact] public void BuildToolErrorLog_IsActionable() { - var message = McpServer.BuildToolErrorLog("search", "bad db"); + var message = McpServer.BuildToolErrorLog("search", new InvalidOperationException("bad db")); - Assert.Contains("Tool error (search): bad db", message); + Assert.Contains("Tool error (search): InvalidOperationException", message); Assert.Contains("Fix the tool arguments", message); Assert.Contains("refresh the index if needed", message); Assert.Contains("retry", message); @@ -3149,6 +3173,55 @@ public async Task StdioTransport_Utf16BomInput_ThrowsDecodeFailure() await Assert.ThrowsAsync(() => transport.ReadFrameAsync(CancellationToken.None)); } + [Fact] + public async Task StdioTransport_ReadFrameAsync_RejectsOversizedLineWhileReading_Issue3506() + { + await using var input = new MemoryStream(Encoding.UTF8.GetBytes("abcdef\n")); + await using var output = new MemoryStream(); + await using var transport = new StdioMcpTransport( + input, + output, + bufferSize: 2, + maxLineCharacters: 5, + maxLineUtf8Bytes: 100); + + var ex = await Assert.ThrowsAsync(() => transport.ReadFrameAsync(CancellationToken.None)); + + Assert.Equal(6, ex.CharactersRead); + Assert.Equal(5, ex.MaxCharacters); + } + + [Fact] + public async Task RunAsync_StdioOversizedFrame_ReturnsMessageTooLarge_Issue3506() + { + await using var input = new MemoryStream(Encoding.UTF8.GetBytes("abcdef\n")); + await using var output = new MemoryStream(); + await using var transport = new StdioMcpTransport( + input, + output, + bufferSize: 2, + maxLineCharacters: 5, + maxLineUtf8Bytes: 100); + using var server = new McpServer(_dbPath, ConsoleUi.LoadVersion()); + using var error = new StringWriter(); + var previousError = Console.Error; + Console.SetError(error); + try + { + await server.RunAsync(transport, CancellationToken.None); + } + finally + { + Console.SetError(previousError); + } + + var raw = Encoding.UTF8.GetString(output.ToArray()); + using var response = JsonDocument.Parse(raw); + Assert.Equal(-32700, response.RootElement.GetProperty("error").GetProperty("code").GetInt32()); + Assert.Equal("message_too_large", response.RootElement.GetProperty("error").GetProperty("data").GetProperty("category").GetString()); + Assert.Contains("Message too large", error.ToString(), StringComparison.Ordinal); + } + [Fact] public async Task StdioTransport_WriteFrameAsync_FlushesBeforeReturning() { @@ -8866,6 +8939,34 @@ public void ToolsCall_BatchQuery_SanitizesSlotExceptionMessage_Issue2849() } } + [Fact] + public void BuildToolErrorLog_SuppressesRawExceptionMessage_Issue3370() + { + const string secret = "SECRET_TOOL_LOG_3370"; + + var log = McpServer.BuildToolErrorLog("search", new InvalidOperationException($"near '{secret}': syntax error")); + + Assert.Contains("InvalidOperationException", log, StringComparison.Ordinal); + Assert.DoesNotContain(secret, log, StringComparison.Ordinal); + Assert.DoesNotContain("syntax error", log, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public void ToolsCall_FindInFileInvalidRegex_DoesNotEchoRegexExceptionMessage_Issue3370() + { + const string secret = "SECRET_REGEX_3370"; + var request = JsonNode.Parse( + "{\"jsonrpc\":\"2.0\",\"id\":3370,\"method\":\"tools/call\",\"params\":{\"name\":\"find_in_file\",\"arguments\":{\"path\":\"src/app.cs\",\"query\":\"(?<" + + secret + + "\",\"regex\":true}}}")!; + + var response = _server.HandleMessage(request)!; + + var error = response["result"]!["content"]![0]!["text"]!.GetValue(); + Assert.Equal("invalid regular expression. Check regex syntax and retry.", error); + Assert.DoesNotContain(secret, error, StringComparison.Ordinal); + } + [Fact] public void ToolsCall_BatchQuery_RejectsTypeMismatchedInnerArguments_Issue1615() { diff --git a/tests/CodeIndex.Tests/ProgramRunnerTests.cs b/tests/CodeIndex.Tests/ProgramRunnerTests.cs index 5e3259bfc1..4585442b35 100644 --- a/tests/CodeIndex.Tests/ProgramRunnerTests.cs +++ b/tests/CodeIndex.Tests/ProgramRunnerTests.cs @@ -57,13 +57,36 @@ public void ResolveMcpHttpBearerTokenFromEnvironment_HttpTokenWinsThenFallsBackT env.Set(McpAuthenticatorFactory.AuthTokenEnvVar, "generic-secret"); Assert.Equal("http-secret", ProgramRunner.ResolveMcpHttpBearerTokenFromEnvironment()); - env.Set(ProgramRunner.McpHttpTokenEnvVar, " "); + env.Set(ProgramRunner.McpHttpTokenEnvVar, string.Empty); Assert.Equal("generic-secret", ProgramRunner.ResolveMcpHttpBearerTokenFromEnvironment()); - env.Set(McpAuthenticatorFactory.AuthTokenEnvVar, "\t"); + env.Set(McpAuthenticatorFactory.AuthTokenEnvVar, string.Empty); Assert.Null(ProgramRunner.ResolveMcpHttpBearerTokenFromEnvironment()); } + [Theory] + [InlineData(" http-secret")] + [InlineData("http-secret ")] + [InlineData("http secret")] + [InlineData("http-secret\n")] + public void ResolveMcpHttpBearerTokenFromEnvironment_RejectsWhitespaceOrControlToken_Issue3505(string token) + { + using var env = EnvironmentVariableScope.Capture( + ProgramRunner.McpHttpTokenEnvVar, + McpAuthenticatorFactory.AuthTokenEnvVar); + env.Set(ProgramRunner.McpHttpTokenEnvVar, token); + env.Set(McpAuthenticatorFactory.AuthTokenEnvVar, "generic-secret"); + + var ex = Assert.Throws(ProgramRunner.ResolveMcpHttpBearerTokenFromEnvironment); + Assert.Contains(ProgramRunner.McpHttpTokenEnvVar, ex.Message, StringComparison.Ordinal); + + env.Set(ProgramRunner.McpHttpTokenEnvVar, null); + env.Set(McpAuthenticatorFactory.AuthTokenEnvVar, token); + + ex = Assert.Throws(ProgramRunner.ResolveMcpHttpBearerTokenFromEnvironment); + Assert.Contains(McpAuthenticatorFactory.AuthTokenEnvVar, ex.Message, StringComparison.Ordinal); + } + [Fact] public void CreateMcpAuthenticatorForTransport_HttpUsesBearerGateInsteadOfBodyTokenGate() { diff --git a/tests/CodeIndex.Tests/QueryCommandRunnerTests.cs b/tests/CodeIndex.Tests/QueryCommandRunnerTests.cs index cf878f044b..787e61e07e 100644 --- a/tests/CodeIndex.Tests/QueryCommandRunnerTests.cs +++ b/tests/CodeIndex.Tests/QueryCommandRunnerTests.cs @@ -837,6 +837,32 @@ public void RunBatch_LineExceedsLimit_SkipsParsingAndContinues_Issue2891() } } + [Fact] + public void RunBatch_InvalidJsonDoesNotEchoParserMessage_Issue3425() + { + const string secret = "SECRET_BATCH_JSON_3425"; + var projectRoot = TestProjectHelper.CreateTempProject("cdidx_batch_invalid_json"); + try + { + var dbPath = TestProjectHelper.CreateProjectDb(projectRoot); + var input = "[\"status\", " + secret + "\n"; + + var (exitCode, stdout, stderr) = CaptureConsoleWithInput( + input, + () => QueryCommandRunner.RunBatch(["--db", dbPath], _jsonOptions)); + + Assert.Equal(CommandExitCodes.UsageError, exitCode); + Assert.Equal(string.Empty, stdout); + Assert.Contains("invalid_batch_json: JsonException", stderr, StringComparison.Ordinal); + Assert.DoesNotContain(secret, stderr, StringComparison.Ordinal); + Assert.DoesNotContain("not valid JSON", stderr, StringComparison.OrdinalIgnoreCase); + } + finally + { + TestProjectHelper.DeleteDirectory(projectRoot); + } + } + [Fact] public void RunBatch_ArgumentCountExceedsLimit_ReturnsUsageError_Issue2891() { @@ -931,7 +957,7 @@ public void RunBatch_TooDeepJsonLine_ReturnsUsageError_Issue3022() Assert.Equal(CommandExitCodes.UsageError, exitCode); Assert.Equal(string.Empty, stdout); - Assert.Contains("is not valid JSON", stderr); + Assert.Contains("invalid_batch_json: JsonException", stderr); } finally {